-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(sveltekit): Align error status filtering and mechanism in handleErrorWithSentry
#17157
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
fix(sveltekit): Align error status filtering and mechanism in handleErrorWithSentry
#17157
Conversation
}; | ||
} | ||
|
||
// 4xx are expected errors and thus we don't want to capture them | ||
function is4xxError(input: SafeHandleServerErrorInput): boolean { |
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.
Bug: Sentry Error Handling Breaks SvelteKit 1.x Compatibility
The handleErrorWithSentry
function in both client and server modules introduces a type mismatch and breaks SvelteKit 1.x backwards compatibility. The input
parameter's type was changed from SafeHandleServerErrorInput
(where status
and message
are optional) to HandleServerErrorInput
/HandleClientErrorInput
(where they are required). This causes a TypeScript error and prevents SvelteKit 1.x from calling the function, as it may not provide these properties. The is4xxError
helper function still expects an optional status
, and existing @ts-expect-error
comments confirm that 1.x compatibility is still expected.
Locations (2)
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.
This is incorrect. I did make the type change, to get rid of two/four ts-expect-error
pragmas but we still use SafeHandle(Sever|Client)ErrorInput
in the is4xxError
checks respectively.
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.
Also, there's no kit@1 breakage because we just bail if we don't have a status on the client. On the server, we use the same fallback mechanism for detecting 404/"Not Found" errors but this isn't relevant for the client.
…ErrorWithSentry` (#17157) Our `handleErrorWithSentry` logic was diverging a bit from the rest of the SvelteKit SDK and more general SDK patterns. Specifically, which errors would be ignored and when we set an error mechanism to `handled: true|false`. This PR now aligns two behaviours: - Ignore all 4xx errors (previously, we only ignored 404 errors) - Set `handled: true` if a custom error handler was defined by users and `handled: false` otherwise.
Our
handleErrorWithSentry
logic was diverging a bit from the rest of the SvelteKit SDK and more general SDK patterns. Specifically, which errors would be ignored and when we set an error mechanism tohandled: true|false
.This PR now aligns two behaviours:
handled: true
if a custom error handler was defined by users andhandled: false
otherwise.Reported via Support, so no GH issue.