Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/backend/src/polyfills.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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") => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The level parameter is typed inline as "error" | "warning" rather than using the exported CaptureLevel type from errors.tsx. If CaptureLevel gains a new variant in the future, this sink silently drops those events without a compile error. Importing and using CaptureLevel keeps the sink in sync with the shared contract.

Suggested change
const sentryErrorSink = (location: string, error: unknown, level: "error" | "warning") => {
const sentryErrorSink = (location: string, error: unknown, level: import("@stackframe/stack-shared/dist/utils/errors").CaptureLevel) => {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/polyfills.tsx
Line: 13

Comment:
The `level` parameter is typed inline as `"error" | "warning"` rather than using the exported `CaptureLevel` type from `errors.tsx`. If `CaptureLevel` gains a new variant in the future, this sink silently drops those events without a compile error. Importing and using `CaptureLevel` keeps the sink in sync with the shared contract.

```suggestion
const sentryErrorSink = (location: string, error: unknown, level: import("@stackframe/stack-shared/dist/utils/errors").CaptureLevel) => {
```

How can I resolve this? If you propose a fix, please make it concise.

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 });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Prompt To Fix With AI
This 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());
};

Expand Down
4 changes: 2 additions & 2 deletions apps/dashboard/src/polyfills.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ function expandStackPortPrefix(value?: string | null) {
return prefix ? value.replace(/\$\{NEXT_PUBLIC_HEXCLAVE_PORT_PREFIX:-81\}/g, prefix as string) : value;
}

const sentryErrorSink = (location: string, error: unknown) => {
Sentry.captureException(error, { extra: { location } });
const sentryErrorSink = (location: string, error: unknown, level: "error" | "warning") => {
Sentry.captureException(error, { extra: { location }, level });
};

export function ensurePolyfilled() {
Expand Down
4 changes: 2 additions & 2 deletions packages/stack-cli/src/lib/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ export function initSentry() {
},
});

registerErrorSink((location, error) => {
Sentry.captureException(error, { extra: { location } });
registerErrorSink((location, error, level) => {
Sentry.captureException(error, { extra: { location }, level });
ignoreUnhandledRejection(Sentry.flush(2000));
});
}
48 changes: 34 additions & 14 deletions packages/stack-shared/src/utils/errors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,28 +94,46 @@ export function errorToNiceString(error: unknown): string {
}


const errorSinks = new Set<(location: string, error: unknown, ...extraArgs: unknown[]) => void>();
export function registerErrorSink(sink: (location: string, error: unknown) => void): void {
export type CaptureLevel = "error" | "warning";

type ErrorSink = (location: string, error: unknown, level: CaptureLevel, ...extraArgs: unknown[]) => void;

const errorSinks = new Set<ErrorSink>();
export function registerErrorSink(sink: ErrorSink): void {
if (errorSinks.has(sink)) {
return;
}
errorSinks.add(sink);
}
registerErrorSink((location, error, ...extraArgs) => {
console.error(
`\x1b[41mCaptured error in ${location}:`,
registerErrorSink((location, error, level, ...extraArgs) => {
const logger = level === "warning" ? console.warn : console.error;
const colorCode = level === "warning" ? "\x1b[43m" : "\x1b[41m";
const label = level === "warning" ? "warning" : "error";
logger(
`${colorCode}Captured ${label} in ${location}:`,
// HACK: Log a nicified version of the error to get around buggy Next.js pretty-printing
// https://www.reddit.com/r/nextjs/comments/1gkxdqe/comment/m19kxgn/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button
errorToNiceString(error),
...extraArgs,
"\x1b[0m",
);
});
registerErrorSink((location, error, ...extraArgs) => {
registerErrorSink((location, error, level, ...extraArgs) => {
globalVar.stackCapturedErrors = globalVar.stackCapturedErrors ?? [];
globalVar.stackCapturedErrors.push({ location, error, extraArgs });
globalVar.stackCapturedErrors.push({ location, error, level, extraArgs });
});

function dispatchToSinks(location: string, error: unknown, level: CaptureLevel): void {
for (const sink of errorSinks) {
sink(
location,
error,
level,
...error && (typeof error === 'object' || typeof error === 'function') && "customCaptureExtraArgs" in error && Array.isArray(error.customCaptureExtraArgs) ? (error.customCaptureExtraArgs as any[]) : [],
);
}
}

/**
* Captures an error and sends it to the error sinks (most notably, Sentry). Errors caught with captureError are
* supposed to be seen by an engineer, so they should be actionable and important.
Expand All @@ -126,13 +144,15 @@ registerErrorSink((location, error, ...extraArgs) => {
* Errors that bubble up to the top of runAsynchronously or a route handler are already captured with captureError.
*/
export function captureError(location: string, error: unknown): void {
for (const sink of errorSinks) {
sink(
location,
error,
...error && (typeof error === 'object' || typeof error === 'function') && "customCaptureExtraArgs" in error && Array.isArray(error.customCaptureExtraArgs) ? (error.customCaptureExtraArgs as any[]) : [],
);
}
dispatchToSinks(location, error, "error");
}

/**
* Like captureError, but logs at warning level. Use for issues that an engineer should know about but that aren't
* severe enough to be treated as an error (e.g. recoverable failures in background tasks).
*/
export function captureWarning(location: string, error: unknown): void {
dispatchToSinks(location, error, "warning");
}


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { isBrowserLike } from "@stackframe/stack-shared/dist/utils/env";
import { captureWarning } from "@stackframe/stack-shared/dist/utils/errors";
import { runAsynchronously } from "@stackframe/stack-shared/dist/utils/promises";
import { Result } from "@stackframe/stack-shared/dist/utils/results";

Expand Down Expand Up @@ -255,12 +256,12 @@ export class SessionRecorder {
);

if (res.status === "error") {
console.warn("SessionRecorder flush failed:", res.error);
captureWarning("SessionRecorder.flush", res.error);
return;
}

if (!res.data.ok) {
console.warn("SessionRecorder flush failed:", res.data.status, await res.data.text());
captureWarning("SessionRecorder.flush", new Error(`SessionRecorder flush failed: ${res.data.status} ${await res.data.text()}`));
}
} finally {
this._flushInProgress = false;
Expand Down
Loading