-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: improve error message for handleHttpError
and handleMissingId
#9621
Conversation
🦋 Changeset detectedLatest commit: 572b018 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
handleHttpError
and handleMissingId
handleHttpError
and handleMissingId
return `${status} ${message}${referrer ? ` (${referenceType} from ${referrer})` : ''}`; | ||
return ( | ||
`${status} ${message}${referrer ? ` (${referenceType} from ${referrer})` : ''}` + | ||
`\nTo suppress or handle this error, implement \`handleHttpError\` in https://kit.svelte.dev/docs/configuration#prerender` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that if the user implements handleHttpError
this will be the message
passed to that function and it might be undesirable to include "To suppress or handle this error..." there as you'll be getting that message even when you do implement the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well now we're full-circle, ha -- what's the solution to the issue if not this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, looking at the signature for the handler, I'm not sure this message would be undesirable -- we pass through the parsed information they need to do actual handling, so hopefully they're not trying to parse the message or something silly like that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree with @tcc-sejohnson - the message is desirable IMO (won't merge for now to let others weigh in)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ben's right, it's weird for this text to show up if the handler was implemented. Whether or not it's likely that someone will use message
in their own handlers is separate to the question of whether the message is correct (but the whole reason it's provided is so that you can use it)
Closes #9592 by adding a line to both prerender error handlers explaining how to suppress or customize their handling.
I wasn't sure where to add a test for this, but happy to add one if someone knows a good location.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.