[fix] Handle 4xx and 5xx statuses without requiring Error instance

[damain]

This PR fixes #1161

It checks if loaded.status is between 400 & 599 and if no loaded.error is sent it creates a message for each status and returns the status and message

It includes messages for the following statuses 400, 401, 403, 404, 409, 500
Any other statuses are defaulted to 500

One failing test for a 401 status is included.
If any other statuses or tests need to be added let me know and I'll get it done.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7792c74

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]

Let's remove all the default error messages (which means you can ignore most of my comments as they'll no longer be relevant). It will make the client bundle too large since they're used on the client:

packages/kit/src/runtime/client/renderer.js:import { normalize } from '../load.js';

Also, you'll need to make Lint pass on the CI and add a changeset

[damain]

Let's remove all the default error messages (which means you can ignore most of my comments as they'll no longer be relevant). It will make the client bundle too large since they're used on the client:

packages/kit/src/runtime/client/renderer.js:import { normalize } from '../load.js';

Also, you'll need to make Lint pass on the CI and add a changeset

Should it have a default error included

return {
    status,
    error: new Error('Error Object Not Specified')
};

[benmccann]

I'd probably not include any type of default error message

[damain]

I am thinking since the status was between 4xx and 5xx an error should exist because the error is needed in order for the node not to load.
Since one was not sent we can do a warn to the console return the status and an empty error

console.warn(' "status" property was returned from load() without an error ')
return {
    status,
    error: new Error()
};

[damain]

I am thinking since the status was between 4xx and 5xx an error should exist because the error is needed in order for the node not to load.
Since one was not sent we can do a warn to the console return the status and an empty error

console.warn(' "status" property was returned from load() without an error ')
return {
    status,
    error: new Error()
};

Any feedback on this?

[benmccann]

Yeah. Could either return empty or just return Internal Server Error

[damain]

I updated the code it was working but the new client test is failing. I will have to work on this some more to get this working.
(The client test from #1355 was also failing)

[damain]

I am running into a sticking point with solving this. On the server it returns the correct status and loads the error template but on the client side it doesn't load the error template unless an error is passed from load.

I also noted that even though the error is passed, response.status() on the client is always 200. However the node itself seems to have the correct status and it can be passed in as props.

At first I thought the issue could be in client/renderer.js where it only checks for error to see if it should generate the error template. I changed all the checks for error to also check status and that did not solve it.

[benmccann]

I added more information on how to debug the tests here: #1900

[damain]

I added more information on how to debug the tests here: #1900

are you sure #1900 is the correct link

[benmccann]

Yes. But now that it's checked in, you can just view the updated README: https://github.com/sveltejs/kit#testing