Incorrect response `status code` when throwing error from `+server.ts`

[daleweaver777]

Describe the bug

When throwing an error from a +server.ts route, the status code in the response header is set to 500 instead of the status code specified in the error() helper. The nearest Error Boundary does render and the error property in the $page store does have the correct status code, however the response header doesn't match.

Reproduction

+server.ts

import { error } from '@sveltejs/kit';
import type { RequestHandler } from './$types';

const shortcodes: { [index: string]: string } = {
	microsoft: 'https://www.microsoft.com',
	google: 'https://www.google.com'
};

export const GET: RequestHandler = ({ params }) => {
	if (!params.shortcode || !shortcodes[params.shortcode]) {
		throw error(404, 'Shortcode not found');
	}

	return Response.redirect(shortcodes[params.shortcode], 301);
};

Response Header

Status Code: 500 Internal Server Error

$page store

{
  error: HttpError {
    name: 'HttpError',
    stack: undefined,
    status: 404,
    message: 'Shortcode Not Found'
  },
  params: { shortcode: 'test' },
  routeId: '[shortcode]',
  status: 500,
  url: URL {
    href: 'http://localhost:5173/test',
    origin: 'http://localhost:5173',
    protocol: 'http:',
    username: '',
    password: '',
    host: 'localhost:5173',
    hostname: 'localhost',
    port: '5173',
    pathname: '/test',
    search: '',
    searchParams: URLSearchParams {},
    hash: ''
  },
  data: {}
}

Logs

No response

System Info

System:
    OS: macOS 12.5
    CPU: (10) arm64 Apple M1 Max
    Memory: 5.49 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.7.0 - /opt/homebrew/bin/node
    npm: 8.16.0 - /opt/homebrew/bin/npm
  Browsers:
    Brave Browser: 104.1.42.95
    Chrome: 104.0.5112.79
    Firefox Developer Edition: 104.0
    Safari: 15.6
    Safari Technology Preview: 16.0
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.64
    @sveltejs/kit: next => 1.0.0-next.417
    svelte: ^3.44.0 => 3.49.0
    vite: ^3.0.0 => 3.0.8

Severity

serious, but I can work around it

Additional Information

No response

[benmccann]

return redirect(301, 'https://kit.svelte.dev');

This looks incorrect. I believe it should be throw redirect(301, 'https://kit.svelte.dev');

Although I suppose that doesn't matter since you'd never get there because of the if (true) above it. Kind of a confusingly written example...

[Conduitry]

This is the intended behavior. throw error() or throw redirect() are intended for use in load functions. In a +server.js endpoint handler, you should just directly return the Response object with whatever error response you want. Any thrown exception results in a 500.

Perhaps there could be a dev mode warning that makes this more clear - but I don't think we want to start having special handling for certain thrown errors in +server.js files.

[daleweaver777]

I made my example a little more generic so it is easy to reproduce. In my implementation I'm doing some logic in the if statement. If that logic fails I need to render an error boundary on the client. If the logic succeeds, I would redirect to a new route based off a lookup array (not included in example).

It's not a typical API route where you are only returning a JSON response. It's a "pageless" route that either redirects if a condition is met, or throws an error.

I also tried this with +page.server.ts with a load function instead of +server.ts, but this required me to have an accompanying +page.svelte. If I didn't include an empty +page.sveltve file, the types wouldn't get generated and import type { PageServerLoad } from './$types'; wouldn't resolve.

[pilcrowonpaper]

@Conduitry The example for server.js seems to contradict the idea that error() is only for load functions?

https://kit.svelte.dev/docs/routing#server

// src/routes/api/random-number/+server.js
import { error } from '@sveltejs/kit';

export function GET({ url }) {
  const min = Number(url.searchParams.get('min') ?? '0');
  const max = Number(url.searchParams.get('max') ?? '1');
 
  const d = max - min;
 
  if (isNaN(d) || d < 0) {
    throw error(400, 'min and max must be numbers, and min must be less than max');
  }
 
  const random = min + Math.random() * d;
 
  return new Response(String(random));
}

[daleweaver777]

Based off this discussion #5783, I believe its intended to be able to throw an error from a +server.ts file and have the nearest error boundary catch it. This does work, but the response header sent to the browser is 500 instead of matching what you throw with the error() helper.

[pilcrowonpaper]

If this is intended, it does bring consistency between page.server.ts and server.ts (like @Rich-Harris pointed out), but it isn't intuitive at all. One would expect that error() inside server.ts will return a response with a status and the error message (even if it's throwing an exception and not a returning a response). At that point, it seems better to leave server.ts alone like @dummdidumm commented on #5783 and not muddy the waters.

[daleweaver777]

I just updated the example under "reproduction" with a more fleshed out example.

Maybe I'm not using +server.ts correctly but I'm trying to achieve what Remix calls "Resource Routes" https://remix.run/docs/en/v1/guides/resource-routes. You can return anything from the server or throw a response for the nearest error boundary to catch.

[dummdidumm]

Since we agreed that error and redirect are valid to throw in +server.js, this should work. However, the obeserved behavior (nearest error page is rendered, page is shown) is also wrong (as a consequence of this bug) in my opinion. Instead you should get a blank page with just the error message. If you are requesting an endpoint directly, it's not something that is part of the page - and you get the same behavior if you do return new Response('Shortcode not found', { status: 404 }).