Proposal: server error middleware

[koenkivits]

fixes #742, fixes #812, fixes #955, fixes #993, fixes #960

First off: I implemented this before I saw #1512 (comment). I'm not sure if it has any chance of getting merged, but I still wanted to share it. I need it for myself anyway, because I need more flexible error handling and I couldn't wait for Kit right now. :)

This PR adds support for custom error middleware. It allows you to define an error handler in src/routes/_error.{js,ts} like so:

export default function(err, req, res, next) {
    handleError(err);

    next(); // fall back to Sapper error handling

    // or:
    next(otherErr); // fall back to Sapper error handling with different error
                    // (allows you to control what error is rendered)

    // ... or don't call next(). Sapper handling will not be invoked.
}

It also centralizes all Sapper handling. This means:

• all errors that are logged or rendered by Sapper's error handling have their stack trace adjusted for the source map (this was only the case for page errors)
• server routes that throw or call next(err) also render a full error page when possible

... and it makes the default error handling respect err.status (and err.statusCode), like both Express and Polka.

This PR solves the same problem as #1512, but:

• it doesn't introduce a 'new' concept. The notion of (err, req, res, next) => void middleware already exists in Express/Polka and should be familiar to most users. This makes it easier to explain and makes it possible to reuse existing error handlers.
• it also improves the default Sapper error handling (or at least I think so)
• no additional config (unless you consider a src/routes/_error file to be config 🙂)

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. -> Not yet, but I'd be more than happy to add these if this PR has any change of getting merged. [Update]: the tests run successfully on my machine, but I see CI fails. I can have a look at that if necessary

Tests

• Run the tests with npm test and lint the project with npm run lint

[benmccann]

I didn't look too closely at this. A lot of the changes here seem generally agreeable. However, we're working on a successor to Sapper called SvelteKit and I'd rather any major changes go there given our limited review time. The code for it is still private at the moment while we work on getting things in a more stable state, but hopefully we'll have more to share before too long.

[koenkivits]

@benmccann I understand, thanks for looking at it anyway! I know about SvelteKit (and am looking forward to it!), but I need this functionality soon for myself and SvelteKit has no public timeline yet. Good to know more details are coming soon. :)

[benmccann]

Thanks again for this! There's a few merge conflicts. SvelteKit is public now, so maybe better to just focus on making sure SvelteKit works as desired?

[koenkivits]

@benmccann I haven't had the time to look at SvelteKit in detail yet, but I think you're right. I'll close this PR for now and will hopefully adapt it for SvelteKit if I have some spare time. :)

[benmccann]

Thanks!

[bamadesigner]

I came up with a simple fix for anyone who is interested/needs something to hold you over until SvelteKit.

I added the status code to the response sent during handle_page(), so when its passed back through middleware you can check response.statusCode inside the middleware function, e.g. if (500 === response.statusCode) equals true when a 500 error has been thrown.

I decided to fork sapper for myself since they might not do any more work here anyway. This is the only change I've made to my fork. Feel free to copy.

https://github.com/bamadesigner/sapper/pull/1/files