Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom error handler #2122

Closed

Conversation

andrew-womeldorf
Copy link
Contributor

@andrew-womeldorf andrew-womeldorf commented Aug 6, 2021

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 pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

This PR replaces handle_error with a new hook handleError. Closes #1857. This will, by default, print errors as was happening before, or allow the user to define their own error handler.

@changeset-bot
Copy link

changeset-bot bot commented Aug 6, 2021

🦋 Changeset detected

Latest commit: a342c35

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

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome. thanks!

documentation/docs/04-hooks.md Outdated Show resolved Hide resolved
documentation/docs/04-hooks.md Outdated Show resolved Hide resolved
@benmccann benmccann added the feature request New feature or request label Aug 9, 2021
@benmccann
Copy link
Member

@Rich-Harris will you have access to global fetch to send the error off to an error reporting service as you desired with this solution?

@andrew-womeldorf
Copy link
Contributor Author

@Rich-Harris will you have access to global fetch to send the error off to an error reporting service as you desired with this solution?

This may be naive of me, but any logging aggregation tool that I've used, I have an agent running on the server which periodically ships the logs from stdout back to the aggregator. The code in this PR, as I've observed, is only running server-side, so I don't think the client should ever have to be pushing logs back to the aggregator. So I think logging to stdout should be sufficient.

packages/kit/src/core/build/index.js Outdated Show resolved Hide resolved
@andrew-womeldorf
Copy link
Contributor Author

I'm not sure about the failing test from the last workflow run. all of the tests pass locally

@benmccann
Copy link
Member

don't worry about it. most of the jobs passed. if it's just one failing then it's a flaky test

@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 11, 2021
@Rich-Harris Rich-Harris mentioned this pull request Aug 13, 2021
5 tasks
@Rich-Harris
Copy link
Member

Thank you — made some tweaks in #2193, will close this in favour of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Propagate SSR errors
3 participants