fix: handle remote query redirects#15222
Conversation
🦋 Changeset detectedLatest commit: 45c99a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…thub.com/Antonio-Bennett/kit into workaround/query-redirect-error-handling
|
This isn't really the correct solution here. The problem is that somewhere in Svelte or SvelteKit, when a promise is created and stashed, it's not having a So basically there's somewhere we're missing a |
|
Yeah def thought so as well. Just put this out for an immediate workaround for now. Will keep digging around to see if I can find where that catch isn't being added! :) |
Co-authored-by: Elliott Johnson <hello@ell.iott.dev>
|
@elliott-with-the-longest-name-on-github thanks for your feedback and review! Committed the change :) |
teemingc
left a comment
There was a problem hiding this comment.
Could we add a test for this?
|
@teemingc added, is it satisfactory? |
|
@teemingc Hey sorry don't want to be a bother, know you have a lot on your plate just wondering if you could take a look in case this slipped by |
|
So sorry about that! It definitely did slip by 🤦🏼 It's 1 AM here so I'll take a look at this tomorrow. Thanks! |
|
I was a little confused when the test still passed even when I commented out the fix. Turns out the console.log from the original issue is caused by us passing the redirect error to the I've cleaned it up a bit so that the test does fail if a |
|
@teemingc thanks for taking a look and bringing it across the finish line! 🐐 |
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 main, this PR will be updated. # Releases ## @sveltejs/adapter-netlify@6.0.0 ### Major Changes - breaking: `platform.context` is now the [modern Netlify Functions (\[#15203\](#15203)) context](https://docs.netlify.com/build/functions/api/#netlify-specific-context-object) Previously, this was the [AWS Lambda-style context](https://github.com/netlify/primitives/blob/c1ae30f2745f0a73e26e83334695e205a04ab47d/packages/functions/prod/src/function/handler_context.ts). If you were using this in your app (unlikely), you will need to update your code to read from new fields. ### Minor Changes - feat: Migrate to the modern Netlify Functions API ([#15203](#15203)) The Netlify adapter now generates "v2" Netlify Functions, which uses modern standards (ESM, `Request`, `Response`) instead of the legacy "Lambda-compatible" or "v1" format. Under the hood, this greatly simplifies the adapter code and improves maintainability. For more details on features this unlocks for your SvelteKit app, see <https://developers.netlify.com/guides/migrating-to-the-modern-netlify-functions/>. - feat: allow configuring redirects in `netlify.toml` ([#15203](#15203)) The limitation of only being able to configure redirects via the `_redirects` file has been removed. ### Patch Changes - fix: populate `App.Platform` with `context` property ([#15203](#15203)) - Updated dependencies \[[`37293a5`](37293a5), [`5d05ca6`](5d05ca6), [`ed69b77`](ed69b77), [`b1fc959`](b1fc959), [`159aece`](159aece), [`c690579`](c690579), [`dc8cf2d`](dc8cf2d), [`ace2116`](ace2116), [`0f38f49`](0f38f49)]: - @sveltejs/kit@2.51.0 ## @sveltejs/kit@2.51.0 ### Minor Changes - feat: add `scroll` property to `NavigationTarget` in navigation callbacks ([#15248](#15248)) Navigation callbacks (`beforeNavigate`, `onNavigate`, and `afterNavigate`) now include scroll position information via the `scroll` property on `from` and `to` targets: - `from.scroll`: The scroll position at the moment navigation was triggered - `to.scroll`: In `beforeNavigate` and `onNavigate`, this is populated for `popstate` navigations (back/forward) with the scroll position that will be restored, and `null` for other navigation types. In `afterNavigate`, this is always the final scroll position after navigation completed. This enables use cases like animating transitions based on the target scroll position when using browser back/forward navigation. - feat: `hydratable`'s injected script now works with CSP ([#15048](#15048)) ### Patch Changes - fix: put preloads before styles ([#15232](#15232)) - fix: suppress false-positive inner content warning when children prop is forwarded to a child component ([#15269](#15269)) - fix: `fetch` not working when URL is same host but different than `paths.base` ([#15291](#15291)) - fix: navigate to hash link when base element is present ([#15236](#15236)) - fix: avoid triggering `handleError` when redirecting in a remote function ([#15222](#15222)) - fix: include `test` directory in generated `tsconfig.json` alongside existing `tests` entry ([#15254](#15254)) - fix: generate `tsconfig.json` using the value of `kit.files.src` ([#15253](#15253)) ## @sveltejs/adapter-cloudflare@7.2.7 ### Patch Changes - fix: error if `_routes.json` is in the `/static` public directory ([#12821](#12821)) - fix: correctly handle pathnames found in the `_redirects` file ([#12821](#12821)) - Updated dependencies \[[`37293a5`](37293a5), [`5d05ca6`](5d05ca6), [`ed69b77`](ed69b77), [`b1fc959`](b1fc959), [`159aece`](159aece), [`c690579`](c690579), [`dc8cf2d`](dc8cf2d), [`ace2116`](ace2116), [`0f38f49`](0f38f49)]: - @sveltejs/kit@2.51.0 ## @sveltejs/adapter-node@5.5.3 ### Patch Changes - fix: validate `ORIGIN` env var at startup ([#15045](#15045)) - chore(deps): update dependency `@rollup/plugin-commonjs` to v29 ([#14856](#14856)) - Updated dependencies \[[`37293a5`](37293a5), [`5d05ca6`](5d05ca6), [`ed69b77`](ed69b77), [`b1fc959`](b1fc959), [`159aece`](159aece), [`c690579`](c690579), [`dc8cf2d`](dc8cf2d), [`ace2116`](ace2116), [`0f38f49`](0f38f49)]: - @sveltejs/kit@2.51.0 ## @sveltejs/enhanced-img@0.10.1 ### Patch Changes - fix: replace erroneous `import.meta.DEV` with `import.meta.env.DEV` in generated code ([#15285](#15285)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
onrejectedhandler when calling the async SSR render thenable to mark the underlying render promise as handledRedirectthrown during render as a real redirect response inrender_pageContext
The async render path returns a thenable from Svelte’s server renderer. Calling
.thenwithout anonrejectedmeans the underlying async render promise can reject (e.g.Redirect) before any handler is attached, which triggers Node’s unhandled rejection detection. Passing anonrejectedhandler attaches a rejection handler directly to the underlying promise.Separately, when a
Redirectis thrown during render (e.g. via a remotequeryused in a component), it currently falls into the generic error handler and returns a 500. HandlingRedirectinrender_pageensures the response redirects instead.How This Relates To Svelte
The unhandled rejection itself originates in Svelte’s SSR runtime (
Renderer.run()does not attach a rejection handler or tie the async work torenderer.promise). The Svelte PR below addresses that root cause; this Kit PR handles the redirect response once the render error propagates.Notes
Related
querywithredirectproduce 500undefinederror #14883Test Plan
queryduring render no longer triggers unhandled rejection and now redirects properlyPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits