fix: handle rejected streamed data race condition#16268
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/456e37877955e14003d12a30473b398375c67a98Open in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
🦋 Changeset detectedLatest commit: 456e378 The changes in this PR will be included in the next version bump. This PR includes no changesetsWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
See #9785 (comment) |
|
Actually, this PR targets a different problem than the aforementioned issue (at least the failing test is failing) |
teemingc
changed the title
|
Thanks for taking another look, and for reopening/renaming this. I updated the PR description so it no longer says Happy to adjust the scope or wording further if you would prefer a different framing. |
|
Do you mind rebasing this on the |
83f2ce2 to
456e378
Compare
|
Thanks — rebased onto I resolved the rebase conflict by keeping the current Local validation after the rebase:
CI is rerunning on the rebased branch now. |
You’re very welcome, and thank you for the review and merge. We’re big fans of SvelteKit and were happy to help. |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to version-3, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `version-3` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `version-3`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @sveltejs/[email protected] ### Major Changes - breaking: `handle`'s `resolve` is now typed to always return a `Promise` ([#16352](#16352)) - breaking: replace the `$lib` alias with `#lib` and remove `files.lib` config. ([#16360](#16360)) - breaking: disallow cross-origin form submissions without a `Content-Type` header ([#16347](#16347)) - breaking: Server-only directories (`/server/` in the path) are now treated as server-only everywhere inside the project (except `src/routes` and the assets directory) ([#16360](#16360)) - breaking: delegate CORS handling to Vite for static directory requests during development ([#16357](#16357)) ### Minor Changes - feat: reinstate `$env/static/private`, `$env/dynamic/private`, `$env/static/public`, `$env/dynamic/public` and `$app/environment` as deprecated aliases for `$app/env/private` `$app/env/public` and `$app/env` ([#16334](#16334)) ### Patch Changes - perf: cache the default cookie header parse and avoid allocations in `cookies.get` ([#16341](#16341)) - fix: avoid client-side code being bundled by Cloudflare Wrangler ([#16364](#16364)) - fix: handle rejected streamed server data after delayed loads ([#16268](#16268)) - fix: enable CSRF protection in builds with a non-production `NODE_ENV` value ([#16313](#16313)) ## @sveltejs/[email protected] ### Minor Changes - feat: transform import aliases into relative imports in files ([#16360](#16360)) ## @sveltejs/[email protected] ### Patch Changes - fix: correctly bundle entrypoints on Windows ([#16367](#16367)) - Updated dependencies [[`c1ee782`](c1ee782), [`1a1b3ea`](1a1b3ea), [`6423d98`](6423d98), [`6d1f4f0`](6d1f4f0), [`5ca9906`](5ca9906), [`b148d31`](b148d31), [`5ca9906`](5ca9906), [`9f3d9bb`](9f3d9bb), [`7bfd922`](7bfd922), [`ffa0e3b`](ffa0e3b)]: - @sveltejs/[email protected] ## @sveltejs/[email protected] ### Patch Changes - chore: replace the `$lib` alias with `#lib` in docs ([#16360](#16360)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
Related to #9785, but this PR targets a distinct streamed-data rejection race condition demonstrated by the added regression test.
Summary
Thanks for SvelteKit. I really appreciate the care that goes into this project.
This PR fixes a timing issue in streamed server data handling. Previously, the JSON data response waited for all server load promises to settle before serializing any node. If one load returned a rejected streamed promise while another load was still pending, the rejection could occur before SvelteKit had attached the stream rejection handler.
The change creates the JSON data serializer earlier and adds each node as its load result resolves, while still waiting for all nodes before emitting the final data response. That preserves the response shape/order but attaches stream handlers earlier.
I also added a regression route/test where one server load delays serialization and another returns a rejected streamed promise.
Validation
Passed:
pnpm exec prettier --check ...on touched filespnpm --dir packages/kit checkpnpm --dir packages/kit/test/apps/basics checkpnpm run lintpnpm run checkI also ran
pnpm test:kitlocally with reduced workers/retries. The new regression passed inside that run, but the full command failed on an existing unrelated no-SSR dev test (SPA mode / no SSR › cannot use browser-only global on page because of ssr config in +page.js). I did not change that area.Disclosure: I used AI-assisted coding tools while preparing this PR. I reviewed the changes myself, tested them, and take responsibility for the implementation and any follow-up revisions needed.