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

fix(remix): Wrap handleDocumentRequest functions. #5387

Merged
merged 2 commits into from
Jul 8, 2022

Conversation

onurtemizkan
Copy link
Collaborator

@onurtemizkan onurtemizkan commented Jul 7, 2022

Fixes: #5362

We're currently wrapping action and loader functions of Remix routes for tracing and error capturing.

When testing the case in #5362, I realized the render function of a SSR component in Remix has another handler
handleDocumentRequest which doesn't re-throw internal errors so we can't catch them in wrapRequestHandler.

Also added a tracing span for handleDocumentRequest.

Note: This is also the case with resource requests. Maybe we need to wrap them as well? 👇

handleResourceRequest is not passed as a parameter to createRequestHandler (that's how we currently access the handlers to wrap). It also only runs action or loader functions, which are already instrumented. So the errors are captured on action/loader level.

@onurtemizkan onurtemizkan added this to the Sentry Remix SDK milestone Jul 7, 2022
@onurtemizkan onurtemizkan self-assigned this Jul 7, 2022
@AbhiPrasad
Copy link
Member

I think we can also try wrapping resource requests. I will update the main tracking GH issue accordingly.

will review in a bit!

@AbhiPrasad AbhiPrasad mentioned this pull request Jul 7, 2022
26 tasks
@onurtemizkan onurtemizkan force-pushed the onur/remix-wrap-document-requests branch from e765a59 to b3ad7ea Compare July 8, 2022 14:22
@onurtemizkan onurtemizkan force-pushed the onur/remix-wrap-document-requests branch from b3ad7ea to 9ee8190 Compare July 8, 2022 14:28
@AbhiPrasad AbhiPrasad enabled auto-merge (squash) July 8, 2022 14:38
@AbhiPrasad AbhiPrasad merged commit 439b9d3 into master Jul 8, 2022
@AbhiPrasad AbhiPrasad deleted the onur/remix-wrap-document-requests branch July 8, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remix SDK invariant is not captured as Issue
2 participants