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

Loader request not cancelled when streaming promises Single Fetch #9788

Open
timvandam opened this issue Jul 23, 2024 · 3 comments
Open

Loader request not cancelled when streaming promises Single Fetch #9788

timvandam opened this issue Jul 23, 2024 · 3 comments

Comments

@timvandam
Copy link

Reproduction

https://stackblitz.com/edit/remix-run-remix-869qzk?file=package.json,app%2Froutes%2F_index.tsx,app%2Froot.tsx

System Info

System:
    OS: macOS 14.3
    CPU: (16) arm64 Apple M3 Max
    Memory: 2.78 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
    pnpm: 9.5.0 - ~/.nvm/versions/node/v20.11.1/bin/pnpm
  Browsers:
    Chrome: 126.0.6478.183
    Safari: 17.3

Used Package Manager

npm

Expected Behavior

I am encountering issues with Single Fetch when a fetcher is called while a loader is still streaming down deferred promises. If I make the loader/action take arbitrarily long (e.g., awaiting a timeout of 10 seconds) and submit a form multiple times, all requests but the last are correctly aborted. However, this does not happen if a promise is being streamed down. This seems like a bug in Single Fetch. In my case this results in issues with optimistic UI, as data returned by useLoaderData is not in sync with the data returned by the last loader invocation.

Actual Behavior

I would expect the loader request to be cancelled

@timvandam timvandam changed the title Inaccurate fetcher state when streaming promises using Single Feth Loader request not cancelled when streaming promises Single Fetch Jul 23, 2024
@brophdawg11
Copy link
Contributor

I think this may need to be addressed in turbo-stream. The signal is being aborted correctly and if you throttle your connection - you will see it turn red in dev tools if you abort quick enough - before the initial response has returned.

In your case, without throttling, the response has already returned by the time you submit again and we abort the signal and thus the fetch has completed with a 200 so it doesn't turn red since the request itself wasn't aborted. I think we need to proxy that signal along into turbo-stream's decode method so we can stop reading from the stream when it aborts as well?

@timvandam
Copy link
Author

Since turbo-stream's decode takes a readable stream, closing this readable stream should be sufficient (+ perhaps catching errors if turbo stream errors when you close in the middle of some value). Since aborting a fetch closes the readable stream I think its fairly straightforward to implement this, however I'm unable to find where RemixBrowser does its fetching to loaders and actions. Is it in some other repo?

@brophdawg11
Copy link
Contributor

Single fetch does it here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants