feat: allow changing response http status code from handleError

[justinba1010]

closes #14442

Allow Custom HTTP Status Codes From Error Handling

Allows custom http status codes from error objects allow for more robust error handling for production SvelteKit users. This is useful if you are using custom error handling, we allow errors to propagate from other services outside of SvelteKit.

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

• Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

[changeset-bot]

🦋 Changeset detected

Latest commit: e1a5954

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Minor

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

[justinba1010]

Thanks @teemingc! Great suggestions, I've applied those diffs

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-14919-svelte.vercel.app/

[teemingc]

PR looks good to me but I think we might have some reservations about whether this is the best solution or if it's better to avoid ending up in the handleError hook in the first place. Will wait for another maintainer to take a look too.

[justinba1010]

PR looks good to me but I think we might have some reservations about whether this is the best solution or if it's better to avoid ending up in the handleError hook in the first place. Will wait for another maintainer to take a look too.

Sounds good, thank you guys! Commited the suggestions just now as well.

[PatrickG]

How about passing a setStatusCode function to the handleError hook?
Or add a setStatusCode function to the RequestEvent (which gets already passed to the handleError hook).

[justinba1010]

How should we handle an invalid http code, it feels like throwing is wrong here, but implied behavior also doesn't feel terribly correct?

[teemingc]

You're not too far from what we're currently doing. See

kit/packages/kit/src/exports/index.js

Lines 74 to 75 in d3fe110

	if ((!BROWSER || DEV) && (isNaN(status) || status < 400 || status > 599)) {
	throw new Error(`HTTP error status codes must be between 400 and 599 — ${status} is invalid`);

The only difference is the isNaN check and the error message format

[justinba1010]

I'll add those tomorrow 👍

[justinba1010]

I don't want to be too opinionated for an example, very open to suggestions, I'd feel partial to the convention we're using for our internal exceptions if you'd like as it's one common in monorepos.

[teemingc]

Or add a setStatusCode function to the RequestEvent (which gets already passed to the handleError hook).

This one might be a bit of a footgun because it leads to situations where more than one statement is setting the HTTP status code in the same flow of execution. For example, in a server endpoint:

export function GET({ setStatusCode }) {
  setStatusCode(501);
  return new Response(null, { status: 201 }); 
}

If we do want to make it a function, it might be better to scope it to the handleError hook.

[justinba1010]

Or add a setStatusCode function to the RequestEvent (which gets already passed to the handleError hook).

This one might be a bit of a footgun because it leads to situations where more than one statement is setting the HTTP status code in the same flow of execution. For example, in a server endpoint:

export function GET({ setStatusCode }) {
  setStatusCode(501);
  return new Response(null, { status: 201 }); 
}

If we do want to make it a function, it might be better to scope it to the handleError hook.

Ahh that's a really good point. If you can give me some pointers on how that should look, I can get started on that tomorrow, had some difficulty implementing there at first.

[teemingc]

Or add a setStatusCode function to the RequestEvent (which gets already passed to the handleError hook).

This one might be a bit of a footgun because it leads to situations where more than one statement is setting the HTTP status code in the same flow of execution. For example, in a server endpoint:

export function GET({ setStatusCode }) {
  setStatusCode(501);
  return new Response(null, { status: 201 }); 
}

If we do want to make it a function, it might be better to scope it to the handleError hook.

Ahh that's a really good point. If you can give me some pointers on how that should look, I can get started on that tomorrow, had some difficulty implementing there at first.

If it's scoped to the server handleError hook, the HandleServerError type would probably change to include a setStatus function. See https://svelte.dev/docs/kit/@sveltejs-kit#HandleServerError

I would still hold off on this until we can get more eyes on whether this is what we want going forward. The team is currently busy with Svelte's async feature.

[teemingc]

Closing in favour of #16162