fix: Show Internal Error when rendering default Error page when a runtime error occurs on a 404 page

[hjhopp]

Fixes #10898

This change will show an error message "Internal Error" when reproducing the issue discussed above.

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:.

[changeset-bot]

🦋 Changeset detected

Latest commit: 91527ab

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	Patch

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

[benmccann]

I'm not sure if this is an exhaustive list or if it'd be possible to create one. Perhaps we could go in the other direction? What the type of the error when a 404 is returned without additional error? Could we check if it's that type?

[eltigerchino]

iirc we don’t have a type for that right now so we'd have to create one

[hjhopp]

It looks like we get to this 500 error from here:

kit/packages/kit/src/runtime/server/page/respond_with_error.js

Line 108 in dbbd4c7

(await handle_error_and_jsonify(event, options, e)).message

The 404 error happens here: https://github.com/sveltejs/kit/blob/dbbd4c7fc1ae2abef584a29a688e2247a80b9792/packages/kit/src/runtime/server/page/respond_with_error.js#L92C63-L92C63

I wondered if perhaps the error message should get handled in that line 108 in respond_with_error.

[eltigerchino]

I think we still want to change the handle_error_and_jsonify method itself because the logic is not foolproof (e.g., an internal error can occur when the route id doesn't exist and so should be able to return the "internal error" message).

kit/packages/kit/src/runtime/server/utils.js

Line 107 in fb611ef

message: event.route.id != null ? 'Internal Error' : 'Not Found'

[eltigerchino]

These are the two places we are throwing a 404 error internally.
The server:

kit/packages/kit/src/runtime/server/respond.js

Line 483 in 5eb2879

error: new Error(`Not found: ${event.url.pathname}`),

and the client:

kit/packages/kit/src/runtime/client/client.js

Line 1009 in 5eb2879

await handle_error(new Error(`Not found: ${url.pathname}`), {

We should also update the client-side error handling that is using a similar logic as the server one we're changing now:

kit/packages/kit/src/runtime/client/client.js

Line 1330 in 5eb2879

/** @type {any} */ ({ message: event.route.id != null ? 'Internal Error' : 'Not Found' })

[hjhopp]

Would something like making those 404 errors a specific type of error, and then checking that the error is that type in handle_error_and_jsonify, only returning "Not Found" if both the route ID is null and the error is a 404 error work?

[eltigerchino]

Would something like making those 404 errors a specific type of error, and then checking that the error is that type in handle_error_and_jsonify, only returning "Not Found" if both the route ID is null and the error is a 404 error work?

Yes, perhaps something akin to

kit/packages/kit/src/runtime/control.js

Lines 22 to 31 in 98e4b8f

	export class Redirect {
	/**
	* @param {300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308} status
	* @param {string} location
	*/
	constructor(status, location) {
	this.status = status;
	this.location = location;
	}
	}

Then we can check the object using instanceof

[dummdidumm]

Thank you!