-
Notifications
You must be signed in to change notification settings - Fork 515
Add captureWarning for warnings #1506
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,13 +10,13 @@ function expandStackPortPrefix(value?: string | null) { | |
| return prefix ? value.replace(/\$\{NEXT_PUBLIC_HEXCLAVE_PORT_PREFIX:-81\}/g, prefix) : value; | ||
| } | ||
|
|
||
| const sentryErrorSink = (location: string, error: unknown) => { | ||
| const sentryErrorSink = (location: string, error: unknown, level: "error" | "warning") => { | ||
| if (!("captureException" in Sentry)) { | ||
| // this happens if somehow this is called outside of a Next.js script (eg. in the Prisma seed.ts), just log and ignore | ||
| console.log("Attempted to capture Sentry error outside of Next.js script, ignoring"); | ||
| return; | ||
| } | ||
| Sentry.captureException(error, { extra: { location } }); | ||
| Sentry.captureException(error, { extra: { location }, level }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both the backend and dashboard polyfills (and the CLI sink) call Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/backend/src/polyfills.tsx
Line: 19
Comment:
**Sentry exception type used for warnings**
Both the backend and dashboard polyfills (and the CLI sink) call `Sentry.captureException` regardless of `level`. When `level` is `"warning"`, this creates an *Exception* event in Sentry rather than a *Message* event, so warnings land in the Issues section alongside real exceptions. For non-Error values passed to `captureWarning`, Sentry will still try to parse them as exception objects. Using `Sentry.captureMessage` for the `"warning"` branch would keep warnings as message events and errors as exception events, matching Sentry's intended semantic split. The same pattern applies in `apps/dashboard/src/polyfills.tsx` and `packages/stack-cli/src/lib/sentry.ts`.
How can I resolve this? If you propose a fix, please make it concise. |
||
| runAsynchronouslyAndWaitUntil(Sentry.flush()); | ||
| }; | ||
|
|
||
|
|
||
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.
levelparameter is typed inline as"error" | "warning"rather than using the exportedCaptureLeveltype fromerrors.tsx. IfCaptureLevelgains a new variant in the future, this sink silently drops those events without a compile error. Importing and usingCaptureLevelkeeps the sink in sync with the shared contract.Prompt To Fix With AI