Skip to content

Consolidate request-handling utils #5599

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

Closed
AbhiPrasad opened this issue Aug 17, 2022 · 1 comment
Closed

Consolidate request-handling utils #5599

AbhiPrasad opened this issue Aug 17, 2022 · 1 comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Package: node Issues related to the Sentry Node SDK Package: remix Issues related to the Sentry Remix SDK Type: Improvement

Comments

@AbhiPrasad
Copy link
Member

Problem Statement

So while I've been reviewing all the nextjs PRs + working on remix, I figured we should probably take some time to consolidate all the utils we have around managing requests and setting domains, perhaps exporting them all from @sentry/node so they can be consumed accordingly.

We now have a couple of steps we always need to do here, and there's probably a bunch I'm missing

  1. monkey-patch the request handler to consume some kind of req (either a fetch API req https://developer.mozilla.org/en-US/docs/Web/API/Request or a express-like req)
  2. Set up domain to prevent scope bleed
  3. extract sentry-trace and baggage from req
  4. Add request data through event processor
  5. extract method from req
  6. Get paramaterized name
  7. Start transaction with op: http.server, name: ${METHOD} ${PARAMETERIZED_NAME}
  8. Automatically capture 5xx as errors
  9. Call original request handler OR call next() in the case of middleware
  10. try-catch the request handler to process errors
  11. patch response end to end transaction and do cleanup

Solution Brainstorm

Ideally the goal is as follows.

We have a common set of utilities that is all called under the hood, so framework instrumentation just calls wrapRequestHandler(frameworkRequestHandler)) and everything should be done.

This way we can also tell folks in docs to just use our utils if they are using a framework we don't support.

This is tech debt - but worthwhile for us to work on since it'll help unblock future Node SDK work (and reduce all the duplication and inconsistency between express handlers, nextjs withSentry, nextjs data fetching instrumentation, and remix.

@AbhiPrasad AbhiPrasad added Type: Improvement Package: node Issues related to the Sentry Node SDK labels Aug 17, 2022
@AbhiPrasad
Copy link
Member Author

image

image

End goal -> let's try to resurrect this when we start working on another node framework sdk

@lobsterkatie lobsterkatie changed the title Consolidate Node utils Consolidate request-handling utils Sep 27, 2022
@lobsterkatie lobsterkatie added Package: nextjs Issues related to the Sentry Nextjs SDK Package: remix Issues related to the Sentry Remix SDK labels Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Package: node Issues related to the Sentry Node SDK Package: remix Issues related to the Sentry Remix SDK Type: Improvement
Projects
None yet
Development

No branches or pull requests

3 participants