Skip to content

🥅 server: fingerprint service errors by name and message #834

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.

Sign up for GitHub

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

Open
cruzdanilo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🥅 server: fingerprint service errors by name and message #834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/bright-falcon-dance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@exactly/server": patch
---

🥅 fingerprint service errors by name and message
36 changes: 6 additions & 30 deletions server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import persona from "./hooks/persona";
import androidFingerprints from "./utils/android/fingerprints";
import appOrigin from "./utils/appOrigin";
import { closeAndFlush as closeSegment } from "./utils/segment";
import ServiceError from "./utils/ServiceError";

import type { UnofficialStatusCode } from "hono/utils/http-status";

Expand Down Expand Up @@ -273,36 +274,11 @@ frontend.use(
app.route("/", frontend);

app.onError((error, c) => {
let fingerprint: string[] | undefined;
if (error instanceof Error) {
const message = error.message
.split("Error:")
.reduce((result, part) => (result ? `${result}Error:${part}` : part.trimStart()), "");
const status = message.slice(0, 3);
const hasStatus = /^\d{3}$/.test(status);
const hasBodyFormat = message.length === 3 || message[3] === " ";
const body = hasBodyFormat && message.length > 3 ? message.slice(4).trim() : undefined;
if (hasStatus && hasBodyFormat) fingerprint = ["{{ default }}", status];
if (hasStatus && hasBodyFormat && body) {
try {
const json = JSON.parse(body) as { code?: unknown; error?: unknown; message?: unknown };
fingerprint = [
"{{ default }}",
status,
...("code" in json
? [String(json.code)]
: typeof json.message === "string"
? [json.message]
: typeof json.error === "string"
? [json.error]
: [body]),
];
} catch {
fingerprint = ["{{ default }}", status, body];
}
}
}
captureException(error, { level: "error", tags: { unhandled: true }, fingerprint });
Comment on lines 276 to -314
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

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

medium

The removal of the complex error message parsing logic is a significant improvement. Relying on ServiceError for structured error data simplifies the onError handler and makes error fingerprinting more robust and maintainable. This change directly addresses the goal of fingerprinting service errors by name and message, making the Sentry events more actionable.

captureException(error, {
level: "error",
tags: { unhandled: true },
fingerprint: error instanceof ServiceError ? ["{{ default }}", error.name, error.message] : undefined,
});
Comment on lines +277 to +281
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

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

medium

This updated captureException call is much cleaner and leverages the ServiceError class effectively. It directly uses error.name and error.message for fingerprinting, which is a more reliable and structured approach than the previous manual parsing.

Comment on lines 276 to +281
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add trailing comma in fingerprint array.

Line 280’s array should include a trailing comma per repo style.

🔧 Proposed fix 
-    fingerprint: error instanceof ServiceError ? ["{{ default }}", error.name, error.message] : undefined,
+    fingerprint: error instanceof ServiceError ? ["{{ default }}", error.name, error.message,] : undefined,
As per coding guidelines "Use trailing commas in arrays, objects, and function arguments for diff-friendliness".
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.onError((error, c) => {
let fingerprint: string[] | undefined;
if (error instanceof Error) {
const message = error.message
.split("Error:")
.reduce((result, part) => (result ? `${result}Error:${part}` : part.trimStart()), "");
const status = message.slice(0, 3);
const hasStatus = /^\d{3}$/.test(status);
const hasBodyFormat = message.length === 3 || message[3] === " ";
const body = hasBodyFormat && message.length > 3 ? message.slice(4).trim() : undefined;
if (hasStatus && hasBodyFormat) fingerprint = ["{{ default }}", status];
if (hasStatus && hasBodyFormat && body) {
try {
const json = JSON.parse(body) as { code?: unknown; error?: unknown; message?: unknown };
fingerprint = [
"{{ default }}",
status,
...("code" in json
? [String(json.code)]
: typeof json.message === "string"
? [json.message]
: typeof json.error === "string"
? [json.error]
: [body]),
];
} catch {
fingerprint = ["{{ default }}", status, body];
}
}
}
captureException(error, { level: "error", tags: { unhandled: true }, fingerprint });
captureException(error, {
level: "error",
tags: { unhandled: true },
fingerprint: error instanceof ServiceError ? ["{{ default }}", error.name, error.message] : undefined,
});
app.onError((error, c) => {
captureException(error, {
level: "error",
tags: { unhandled: true },
fingerprint: error instanceof ServiceError ? ["{{ default }}", error.name, error.message,] : undefined,
});

return c.json({ code: "unexpected error", legacy: "unexpected error" }, 555 as UnofficialStatusCode);
Comment on lines 276 to 282
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Feb 23, 2026

Choose a reason for hiding this comment

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

🚩 Loss of fingerprinting for non-ServiceError status-formatted errors reaching app.onError

The old index.ts app.onError handler parsed error messages matching the pattern NNN body (e.g., "404 Not Found") and fingerprinted them with the extracted status code and optionally parsed JSON fields. The new code only fingerprints ServiceError instances. Any non-ServiceError Error that previously matched the old pattern (e.g., a plain Error('500 Internal Server Error')) will no longer be fingerprinted at the captureException call site. However, the beforeSend in instrument.cjs compensates for errors with a numeric status property. Plain Error objects without a status property that had status-formatted messages will lose their fingerprinting. This is likely acceptable since the codebase has been migrated to use ServiceError for service call failures, but it's a behavioral change.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

});

Expand Down
18 changes: 13 additions & 5 deletions server/instrument.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,28 @@ init({
],
beforeSend: (event, hint) => {
const exception = event.exception?.values?.[0];
if (!exception) return event;
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

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

medium

Adding a check for !exception at the beginning of beforeSend ensures that subsequent logic only runs when there's an actual exception to process, preventing potential undefined errors and improving robustness.

const error = hint.originalException;
if (
error instanceof Error &&
typeof (/** @type {{ status?: unknown }} */ (error).status) === "number" &&
Comment on lines +32 to +35
Copy link
Copy Markdown

@sentry sentry bot Feb 23, 2026

Choose a reason for hiding this comment

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

Bug: The new fingerprinting logic in instrument.cjs checks for error.status, but PandaError uses statusCode, causing these errors to be missed and not grouped correctly in Sentry.
Severity: MEDIUM

Suggested Fix

Update the check in server/instrument.cjs to look for error.statusCode in addition to error.status, or change the PandaError class to use a status property instead of statusCode for consistency.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: server/instrument.cjs#L32-L35

Potential issue: The fingerprinting logic in `server/instrument.cjs` is designed to
create custom Sentry fingerprints for errors that have a numeric `status` property.
However, `PandaError` instances, which are used for card transactions and refunds, are
defined with a `statusCode` property instead. When a `PandaError` is thrown and
captured, it is sent to Sentry without a pre-defined fingerprint. The `beforeSend` hook
then fails to apply a custom fingerprint because its check for `error.status` fails.
This results in `PandaError` events not being grouped correctly in Sentry, hindering
monitoring and debugging efforts.

Did we get this right? 👍 / 👎 to inform future reviews.

!(event.fingerprint && event.fingerprint.length > 1)
) {
event.fingerprint = ["{{ default }}", error.name, error.message];
}
Comment on lines +32 to +39
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

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

medium

This new block correctly identifies errors that have a status property (likely ServiceError instances or similar) and assigns a default fingerprint using error.name and error.message if one isn't already present. This ensures consistent fingerprinting for these types of errors, aligning with the PR's goal.

Comment on lines +33 to +39
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Feb 23, 2026

Choose a reason for hiding this comment

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

🚩 beforeSend fingerprinting applies to all errors with numeric status, not just ServiceError

The new beforeSend logic at server/instrument.cjs:33-38 fingerprints any Error with a numeric status property, which is broader than just ServiceError. This means if any third-party library throws an error with a numeric status (e.g., HTTP client libraries, ORMs), it will also get fingerprinted by ["{{ default }}", error.name, error.message]. If error.message contains high-cardinality data (request IDs, timestamps, UUIDs), this could create excessive unique fingerprints in Sentry, degrading grouping quality. ServiceError controls this via its constructor's parsing logic (falls back to status code for long messages >200 chars at server/utils/ServiceError.ts:26), but other error types have no such protection. This may be intentional to cast a wide net, but worth confirming.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +31 to +39
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove JSDoc cast and add trailing comma.

JSDoc casts are disallowed, and the fingerprint array needs a trailing comma.

🔧 Proposed fix 
-    if (
-      error instanceof Error &&
-      typeof (/** `@type` {{ status?: unknown }} */ (error).status) === "number" &&
-      !(event.fingerprint && event.fingerprint.length > 1)
-    ) {
-      event.fingerprint = ["{{ default }}", error.name, error.message];
-    }
+    if (
+      error instanceof Error &&
+      typeof error.status === "number" &&
+      !(event.fingerprint && event.fingerprint.length > 1)
+    ) {
+      event.fingerprint = ["{{ default }}", error.name, error.message,];
+    }
As per coding guidelines "Use only static analysis annotations (...) do not use JSDoc" and "Use trailing commas in arrays, objects, and function arguments for diff-friendliness".

if (
exception &&
event.fingerprint?.[0] === "{{ default }}" &&
(exception.type === "ContractFunctionExecutionError" ||
exception.type === "ContractFunctionRevertedError" ||
exception.type === "BaseError")
) {
/** @typedef {{ cause?: unknown; data?: { errorName?: string }; reason?: string; signature?: string }} RevertError */
for (
let error = /** @type {RevertError | undefined} */ (hint.originalException);
error;
error = /** @type {RevertError | undefined} */ (error.cause)
let revert = /** @type {RevertError | undefined} */ (hint.originalException);
revert;
revert = /** @type {RevertError | undefined} */ (revert.cause)
) {
const reason = error.data?.errorName ?? error.reason ?? error.signature;
const reason = revert.data?.errorName ?? revert.reason ?? revert.signature;
if (reason) {
exception.type = reason;
break;
Expand Down