Skip to content

🥅 server: handle expected bridge pairing errors #933

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: handle expected bridge pairing errors #933

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/calm-panda-handle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@exactly/server": patch
---

🥅 handle expected bridge pairing errors
3 changes: 2 additions & 1 deletion server/hooks/bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { captureEvent, captureException, setUser } from "@sentry/core";
import createDebug from "debug";
import { and, DrizzleQueryError, eq, isNull } from "drizzle-orm";
import { Hono } from "hono";
import { HTTPException } from "hono/http-exception";
import { validator } from "hono/validator";
import { createHash, createVerify } from "node:crypto";
import { literal, object, parse, picklist, string, unknown, variant } from "valibot";
Expand Down Expand Up @@ -128,7 +129,7 @@ export default new Hono().post(
.where(and(eq(credentials.id, referenceId), isNull(credentials.bridgeId)))
.returning({ account: credentials.account, source: credentials.source })
.then(([updated]) => {
if (!updated) throw new Error("no match found when pairing bridge id");
if (!updated) throw new HTTPException(409, { message: "credential pairing failed" });
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

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

medium

According to the general rules of this repository, when an expected database record is not found, a 5xx-level error is preferred over a 4xx to indicate a system or data integrity issue. Since !updated here could mean the referenceId (obtained from Persona) does not exist in the credentials table, a 500 error might be more appropriate than a 409 Conflict, unless this specific failure is considered a recoverable state conflict rather than a data integrity issue.

References
  1. Throw an error when an expected database record (like a card) is not found. This indicates a system or data integrity issue, not a client-side error that can be fixed by the user, so a 5xx-level error is more appropriate than a 4xx.

Copy link
Copy Markdown

@sentry sentry bot Apr 2, 2026

Choose a reason for hiding this comment

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

Bug: The global app.onError() handler doesn't check for HTTPException. It will catch the new 409 exception, log it, and incorrectly return a 555 status instead of 409.
Severity: MEDIUM

Suggested Fix

Update the global app.onError() handler in server/index.ts to check if the error is an instanceof HTTPException. If it is, re-throw the exception or return its response directly. For all other errors, maintain the existing behavior of calling captureException() and returning a 555 response.

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/hooks/bridge.ts#L132

Potential issue: The code at `server/hooks/bridge.ts:132` now throws an
`HTTPException(409)` for credential pairing failures. While the intent is to return a
409 status code, the global `app.onError()` handler in `server/index.ts` does not
differentiate `HTTPException` from other errors. As a result, when this exception is
thrown in production, it will be caught by the global handler, which will log it to
Sentry via `captureException()` and return a generic 555 error response. This defeats
the purpose of using `HTTPException` and will cause clients to receive an incorrect
status code. The tests pass because they only test the isolated sub-app, not the full
application stack with the global error handler.

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

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Apr 2, 2026

Choose a reason for hiding this comment

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

🚩 Webhook caller behavior may change with 409 vs 500

Bridge's webhook system may treat 4xx and 5xx responses differently — many webhook providers retry on 5xx (transient server errors) but not on 4xx (client errors indicating the request itself is invalid). Changing from 500 to 409 could stop Bridge from retrying these specific webhook deliveries. This appears intentional given the PR's stated goal of handling "expected" pairing errors, but worth confirming that Bridge's retry behavior for 409 is acceptable for these scenarios (credential already paired, or reference-id not found).

Open in Devin Review

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== confirm throw site =="
rg -n -C3 'throw new HTTPException\(409' server/hooks/bridge.ts

echo
echo "== inspect global error handler behavior =="
rg -n -C8 'app\.onError|captureException\(|unexpected error|555 as UnofficialStatusCode' server/index.ts

Repository: exactly/exa

Length of output: 1810

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== find HTTPException import source =="
rg -n 'import.*HTTPException|from.*HTTPException' server/hooks/bridge.ts

echo
echo "== search for HTTPException definition in codebase =="
rg -n 'class HTTPException|export.*HTTPException' --type ts

echo
echo "== check if Hono exports HTTPException =="
rg -n 'HTTPException' package.json

Repository: exactly/exa

Length of output: 203

🌐 Web query:

Hono HTTPException error handling how does it work

💡 Result:

Hono's HTTPException provides a standardized way to throw and handle HTTP errors. Import it from 'hono/http-exception'. To throw an HTTPException: - Basic: new HTTPException(401, { message: 'Unauthorized' }) - Custom response: new HTTPException(401, { res: new Response('Custom', { status: 401 }) }) - With cause: new HTTPException(401, { message: 'Error', cause: originalError }) Throw it in handlers or middleware for validation failures, auth errors, etc. Handling: Use app.onError((err, c) => { if (err instanceof HTTPException) { return err.getResponse; // Returns Response with status and message/res } // Handle other errors, e.g. console.error(err); return c.text('Internal Server Error', 500); }); This catches all unhandled exceptions, including HTTPExceptions from Hono core/middleware. HTTPException extends Error with status, res?, and getResponse method. For structured JSON errors, customize in onError: return c.json({ error: err.message, status: err.status }, err.status); Official docs confirm this mechanism remains current as of 2026.

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== check for other error handlers or middleware before app.onError =="
rg -n 'app\.use.*error|middleware.*error|app\.onError' server/index.ts | head -20

echo
echo "== verify HTTPException is imported in index.ts =="
rg -n 'HTTPException' server/index.ts

Repository: exactly/exa

Length of output: 208

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== check full error handler logic to confirm status extraction won't match 'credential pairing failed' =="
sed -n '284,316p' server/index.ts

Repository: exactly/exa

Length of output: 1501

HTTPException(409) is masked by the global error handler.

Line 132 throws HTTPException(409, ...), but the global error handler in server/index.ts:284–316 only tries to extract a 3-digit status from error.message. Since the message is "credential pairing failed" (not a status code), hasStatus remains false, and the handler unconditionally returns 555 regardless of the thrown status code.

Fix by either:

  1. Importing and checking instanceof HTTPException in app.onError, then using err.getResponse() or c.json(..., err.status) to preserve the intended status, or
  2. Returning a 409 response directly from the handler instead of throwing.

captureEvent({
message: "bridge credential paired",
level: "warning",
Expand Down
8 changes: 4 additions & 4 deletions server/test/hooks/bridge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ describe("bridge hook", () => {
});
});

it("returns 500 when fallback credential already paired", async () => {
it("returns 409 when fallback credential already paired", async () => {
vi.spyOn(database.query.credentials, "findFirst").mockResolvedValueOnce(undefined); // eslint-disable-line unicorn/no-useless-undefined
vi.spyOn(bridge, "getCustomer").mockResolvedValue({
id: "conflict-bridge-id",
Expand All @@ -303,7 +303,7 @@ describe("bridge hook", () => {
json: payload as never,
});

expect(response.status).toBe(500);
expect(response.status).toBe(409);
expect(captureEvent).not.toHaveBeenCalled();
expect(captureException).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -361,7 +361,7 @@ describe("bridge hook", () => {
);
});

it("returns 500 when fallback reference-id has no credential", async () => {
it("returns 409 when fallback reference-id has no credential", async () => {
vi.spyOn(bridge, "getCustomer").mockResolvedValue({
id: "orphan-bridge-id",
email: "orphan@example.com",
Expand All @@ -380,7 +380,7 @@ describe("bridge hook", () => {
json: payload as never,
});

expect(response.status).toBe(500);
expect(response.status).toBe(409);
expect(captureEvent).not.toHaveBeenCalled();
expect(captureException).not.toHaveBeenCalled();
});
Expand Down