[go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throwing AbortController error every response in the Vite dev server when using Remix, Vite, and Cloudflare #10014

Closed
caprolactam opened this issue Sep 22, 2024 · 15 comments
Labels
awaiting release This issue has been fixed and will be released soon bug:unverified runtime:cloudflare vite

Comments

@caprolactam
Copy link

Reproduction

  1. Generate a project using official cloudflare template.
    npx create-remix@latest --template remix-run/remix/templates/cloudflare
  2. Run the app with the remix vite:build and remix vite:dev.
  3. Visit http://localhost:5173, then the app throws an error.

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (12) x64 AMD Ryzen 5 5625U with Radeon Graphics
    Memory: 6.28 GB / 23.35 GB
Binaries:
    Node: 20.16.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.8.2 - C:\Program Files\nodejs\npm.CMD
Browsers:
    Edge: Chromium (128.0.2739.79)
    Internet Explorer: 11.0.22621.3527
npmPackages:
    @remix-run/cloudflare: ^2.12.1 => 2.12.1
    @remix-run/cloudflare-pages: ^2.12.1 => 2.12.1
    @remix-run/dev: ^2.12.1 => 2.12.1
    @remix-run/react: ^2.12.1 => 2.12.1
    vite: ^5.1.0 => 5.4.7

Used Package Manager

npm

Expected Behavior

The app should run without this error.

Actual Behavior

Throwing this error every response.

TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
    at ReadableByteStreamController.close (node:internal/webstreams/readablestream:1156:13)
    at close (/project/workspace/node_modules/react-dom/cjs/react-dom-server.browser.development.js:143:15)
    at flushCompletedQueues (/project/workspace/node_modules/react-dom/cjs/react-dom-server.browser.development.js:6906:9)
    at abort (/project/workspace/node_modules/react-dom/cjs/react-dom-server.browser.development.js:6951:7)
    at EventTarget.listener (/project/workspace/node_modules/react-dom/cjs/react-dom-server.browser.development.js:7000:9)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:826:20)
    at EventTarget.dispatchEvent (node:internal/event_target:761:26)
    at abortSignal (node:internal/abort_controller:371:10)
    at AbortController.abort (node:internal/abort_controller:393:5)
    at EventTarget.abort (node:internal/deps/undici/undici:6279:21) {
  code: 'ERR_INVALID_STATE'
}

Note that the app works fine in Wrangler(wrangler pages dev ./build/client), but this error occurs when using the Vite dev server. Additionally, this error is thrown when using Remix version is 2.12.1, but not with version 2.12.0. I think the changes in PR #9976 are causing this issue.

@shmuli9
Copy link
shmuli9 commented Sep 22, 2024

Getting this too

@WonderPanda
Copy link

I just started working with Remix + Cloudflare on a prototype and immediately ran into this.

@Phoenixmatrix
Copy link

I can confirm getting this issue too. @caprolactam's theory is correct too. I forked the plugin and removed #9976's changes, and everything works as expected without errors.

@IvorySun
Copy link
IvorySun commented Sep 27, 2024

Hello, I am speaking here for the first time, if there is something inappropriate, please let me know in time and thank you.
I've upgraded the project and I'm still getting the error, in every response. Additional information: Initial project using Cloudflare and Remix templates. will encounter the above errors. I copied the previous project to a new folder, and I also got the error above. The original project will not report errors, Thank you again.
The following error message is displayed:
TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
at ReadableByteStreamController.close (node:internal/webstreams/readablestream:1159:13)
at close (G:\myProjects\cloudflare-2\books-admin.yarn_virtual_\react-dom-virtual-8d992f7834\0\cache\react-dom-npm-18.3.1-a805663f38-a752496c19.zip\node_modules\react-dom\cjs\react-dom-server.browser.development.js:143:15)
at flushCompletedQueues (G:\myProjects\cloudflare-2\books-admin.yarn_virtual_\react-dom-virtual-8d992f7834\0\cache\react-dom-npm-18.3.1-a805663f38-a752496c19.zip\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6906:9)
at abort (G:\myProjects\cloudflare-2\books-admin.yarn_virtual_\react-dom-virtual-8d992f7834\0\cache\react-dom-npm-18.3.1-a805663f38-a752496c19.zip\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6951:7)
at AbortSignal.listener (G:\myProjects\cloudflare-2\books-admin.yarn_virtual_\react-dom-virtual-8d992f7834\0\cache\react-dom-npm-18.3.1-a805663f38-a752496c19.zip\node_modules\react-dom\cjs\react-dom-server.browser.development.js:7000:9)
at [nodejs.internal.kHybridDispatch] (node:internal/event_target:816:20)
at AbortSignal.dispatchEvent (node:internal/event_target:751:26)
at abortSignal (node:internal/abort_controller:382:10)
at AbortController.abort (node:internal/abort_controller:404:5)
at AbortSignal.abort (node:internal/deps/undici/undici:9235:14)
at [nodejs.internal.kHybridDispatch] (node:internal/event_target:816:20)
at AbortSignal.dispatchEvent (node:internal/event_target:751:26)
at abortSignal (node:internal/abort_controller:382:10)
at AbortController.abort (node:internal/abort_controller:404:5)
at ServerResponse. (G:\myProjects\cloudflare-2\books-admin.yarn_virtual_@remix-run-dev-virtual-5750952721\0\cache@remix-run-dev-npm-2.12.1-233a55d9cb-5bbdf06ab2.zip\node_modules@remix-run\dev\dist\vite\node-adapter.js:46:40)
at ServerResponse.emit (node:events:519:28)
at ServerResponse.emit (node:domain:488:12)
at emitCloseNT (node:_http_server:1030:10)
at process.processTicksAndRejections (node:internal/process/task_queues:89:21) {
code: 'ERR_INVALID_STATE'
}
"dependencies": {
"@remix-run/cloudflare": "^2.12.1",
"@remix-run/cloudflare-pages": "^2.12.1",
"@remix-run/react": "^2.12.1",
"bcryptjs": "^2.4.3",
"isbot": "^5.1.17",
"react": "^18.3.1",
"react-dom": "^18.3.1"
},
"devDependencies": {
"@cloudflare/workers-types": "^4.20240925.0",
"@eslint/js": "^9.11.1",
"@remix-run/dev": "^2.12.1",
"@remix-run/server-runtime": "^2.12.1",

@caprolactam
Copy link
Author

@IvorySun Try fixing the version of Remix to 2.12.0 instead of ^2.12.1 in package.json.
After that don't forget to run npm install (if you're using npm).

{
  "dependencies": {
    "@remix-run/cloudflare": "2.12.0",
    "@remix-run/cloudflare-pages": "2.12.0",
    "@remix-run/react": "2.12.0",
  },
  "devDependencies": {
    "@remix-run/dev": "2.12.0",
    "@remix-run/server-runtime": "2.12.0",
  }
}

@IvorySun
Copy link
IvorySun commented Sep 27, 2024

hey @caprolactam I am very happy to see your reply, thank you so much. I will try it as you said.
Another important supplementary information. After I copied the original project to the new folder, "dev": "remix vite:dev", an error message appeared: @cloudflare/workerd-windows-64 is required. The previous project did not encounter this situation. Only after adding it separately can dev be run, and then an error will be reported in each response. It is the same as the situation mentioned in this post.
Thank you again for your patience.
I downgraded all 5 related projects to 2.12.0, and the error still occurred, the same as before.

@caprolactam
Copy link
Author
caprolactam commented Sep 27, 2024

Hi @IvorySun the issue you're facing might be more complicated than the bug mentioned in this issue. Unfortunately, I can only provide a workaround related to this bug. Anyway, please double-check the following points. If this doesn't fix it, there might be another problem.

  1. Lock the Remix packages version, not "@remix-run/cloudflare": "^2.12.1", but "@remix-run/cloudflare": "2.12.0". If you include ^ at the beginning of the version, it might apply updates 2.12.1. (You will be able to restore ^ after fixing this bug.)
  2. Ensure both @cloudflare/workers-types and wrangler are up-to-date. The current versions are "@cloudflare/workers-types": "^4.20240925.0" and "wrangler": "3.78.10".

@IvorySun
Copy link

Hi @caprolactam
First of all, thank you very much for your patience.

I updated and locked the version of the relevant package exactly as you said. But the error still occurs.
For testing, I completely copied a previous project, including package.json, but still reported an error.
The previous project is currently normal and has no errors.
I suspect that the problem is caused by wrangler, because wrangler automatically updates the version according to the creation time.
I have raised the issue in the cloudflare community. But I have not received a response so far.

Thank you very much for your enthusiastic response.
If there is any useful progress in the cloudflare community's questions, I will share it with you and everyone in the GitHub community.

Thank you again
Kind regards,
Ivory

@bmatzner
Copy link
bmatzner commented Sep 27, 2024

@IvorySun please make sure the dev package 2.12.0 is indeed installed (by inspecting your package manager lock file). You might have to delete your lock file and run a fresh install to do that. I can confirm using 2.12.0 does not cause the issue.

@IvorySun
Copy link

@bmatzner @caprolactam You are right.
I just tried again,

Lock the Remix packages version, not "@remix-run/cloudflare": "^2.12.1", but "@remix-run/cloudflare": "2.12.0". If you include ^ at the beginning of the version, it might apply updates 2.12.1. (You can restore ^ after fixing this bug.)
Ensure both @cloudflare/workers-types and wrangler are up-to-date. The current versions are "@cloudflare/workers-types": "^4.20240925.0" and "wrangler": "3.78.10".

No errors now.
You are so kind, I appreciate it very much 💯

@raggesilver
Copy link

Getting this on a fresh Cloudflare project as well. Unfortunately pinning cloudflare to 2.12.0 brings back the type single fetch errors which were fixed in 2.12.1...

@joakimsjo
Copy link
joakimsjo commented Sep 30, 2024

We are also seeing this behaviour
cc @jonjohansen

@brophdawg11
Copy link
Contributor

Hm - I guess since these aren't actual Node req/res objects they shouldn't get the same treatment - Cloudflare must be doing something on it's own inside of getPlatformProxy. Maybe we need to call dispose when the node response closes instead of aborting the signal...or maybe we don't have to do anything if they handle it internally.

We'll take a look at this, but in the meantime - @Phoenixmatrix since you forked and removed that change - are you able to confirm that with that changes removed, are requests aborted properly on the server during dev when using cloudflare?

The easiest way to check this is to set up a loader such as the following and then use a fetcher.load to hit it a few times quickly so it interrupts the previous fetches:

export async function loader({ request }: LoaderFunctionArgs) {
  request.signal.addEventListener("abort", () => console.log("signal aborted"));
  await new Promise((r) => setTimeout(r, 1000));
  return { value: Math.random() };
}

@brophdawg11
Copy link
Contributor

ok this turns out to be a two-fold issue. The tl;dr; is that it will be resolved via #10046 and #10047.

The first issue is that our long standing logic for aborting Node requests was slightly flawed. We used to abort requests on the res close event which meant the Node response could no longer be written to. This has the unanticipated impact of also aborting request.signal after successful document responses - but that was not posing any issues since by definition all loaders/actions had completed by then. And more importantly - the request.signal was not coupled to any abort behavior of renderToPipeableStream in the Node entry.server.tsx file. This is fixed by #10046.

The second half of the issue is that cloudflare/deno entry.server.tsx files have always been (incorrectly) passing the request.signal as the signal for renderToReadableStream. When aborted, React tries to flush all remaining boundaries to the client. This was never going to work because if we abort the request when we can lo longer write to the response, then aborting renderToReadableStream will always fail to flush down unresolved boundaries. This never showed it's face before because we were never aborting requests during vite:dev. Once we resolved that issue via #9976, we caused this other unknown bug to show up. The fix here is that the cloudflare/deno entry.server.tsx files should not be coupled to request.signal for aborting rendering - they should instead be timeout driven like the node entry.server.tsx has always been. In #10047 we remove the signal in the default entry.server implementations to avoid this "tell React to flush unresolved boundaries knowing that we can't write to the response" issue. We do not add any sort of default timeout because that would be a breaking change for Remix v2 users who do not have their own entry.server files. We do add an example timeout approach in the renderToReadableStream templates that users can adopt in their own entry.server.tsx files if desired.

It's also worth noting that if you've adopted single fetch, theres a new timeout mechanism for streamed data that you will want to be aware of.

@brophdawg11
Copy link
Contributor

This is resolved by the above PRs and will be available in the next release

@brophdawg11 brophdawg11 removed their assignment Oct 3, 2024
@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release This issue has been fixed and will be released soon bug:unverified runtime:cloudflare vite
Projects
None yet
Development

No branches or pull requests

9 participants