From 9505961f4e3ce29c27894ed6ad40b4394f6ce543 Mon Sep 17 00:00:00 2001
From: Brandon Corbett <Bccorb@users.noreply.github.com>
Date: Mon, 31 Aug 2026 09:51:22 -0400
Subject: [PATCH] fix(errors): report an unhandled error as 500 rather than 404

The first error handler answered the CORS rejection and passed everything else
on with a bare next(). From an error handler that clears the error and resumes
at the next regular middleware, so the 500 handler directly below was skipped
and control landed on the 404. Every unhandled exception was reported to the
caller as 404 Not Found.

The 404 handler also logs a requestSuspicious event, so each internal error was
written into the anomaly signal as suspicious behaviour by whoever sent the
request. Routing errors to the 500 handler takes them out of that stream.

A genuinely unmatched route still answers 404. Surveyed the dependents: the
React SDK does not branch on 404 at all, the server adapter's only reference is
an unrelated comment, and the admin dashboard already maps 5xx to a clearer
message than the 404 text it was getting.
---
 .changeset/quiet-hounds-report.md | 28 ++++++++++++++++++++++++++++
 src/app.ts                        |  6 +++++-
 tests/unit/app.spec.ts            | 15 ++++++++++++---
 3 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/quiet-hounds-report.md

diff --git a/.changeset/quiet-hounds-report.md b/.changeset/quiet-hounds-report.md
new file mode 100644
index 0000000..239a083
--- /dev/null
+++ b/.changeset/quiet-hounds-report.md
@@ -0,0 +1,28 @@
+---
+'seamless-auth-api': minor
+---
+
+Report an unhandled server error as 500 rather than 404.
+
+The first error handler answered the CORS rejection and passed everything else on
+with a bare `next()`. Calling `next()` with no argument from an error handler clears
+the error and resumes at the next **regular** middleware, so the 500 handler directly
+below it was skipped and control landed on the 404. Any unhandled exception was
+therefore reported to the caller as `404 {"error":"Not Found"}`.
+
+Two consequences beyond the wrong status:
+
+- **Server faults were recorded against the caller.** The 404 handler logs an
+  `AuthEventService.requestSuspicious` event with reason "Request to an unknown
+  route." Every internal error was written into the anomaly signal the dashboard and
+  the security views read, as suspicious behaviour by whoever happened to send the
+  request. That stream is now free of them.
+- **Real errors were easy to miss.** A 500 is alertable. A 404 on an auth API is
+  background noise, and this masked a genuine regression through a full test run.
+
+**Behaviour change:** a request that triggers an unhandled exception now answers
+`500 {"error":"Internal server error"}` instead of `404 {"error":"Not Found"}`. A
+genuinely unmatched route still answers 404, unchanged. Callers that retry on 5xx but
+not 4xx will now retry these. No dependent needed changing: the React SDK does not
+branch on 404, and the admin dashboard already maps `>= 500` to a clearer message than
+the 404 text it was getting.
diff --git a/src/app.ts b/src/app.ts
index 34aa7ae..04b6af3 100644
--- a/src/app.ts
+++ b/src/app.ts
@@ -147,7 +147,11 @@ export async function createApp() {
       // is not one tells it part of the allowlist and helps the browser not at all.
       return res.status(403).json({ message: 'CORS policy does not allow this origin.' });
     }
-    return next();
+    // next(err), not next(): a bare next() from an error handler clears the error and
+    // resumes at the next regular middleware, which skips the 500 handler below and lands
+    // on the 404. That reported server faults as unknown routes and recorded the caller
+    // for them.
+    return next(err);
   });
 
   app.use((err: unknown, req: Request, res: Response, next: NextFunction) => {
diff --git a/tests/unit/app.spec.ts b/tests/unit/app.spec.ts
index 797a573..408be29 100644
--- a/tests/unit/app.spec.ts
+++ b/tests/unit/app.spec.ts
@@ -129,11 +129,20 @@ describe('query parsing', () => {
 });
 
 describe('createApp error handling', () => {
-  it('passes non-CORS errors through to the not-found handler', async () => {
+  it('reports a non-CORS error as a server error', async () => {
     const res = await request(built).get('/__test_plain_error');
 
-    expect(res.status).toBe(404);
-    expect(res.body).toEqual({ error: 'Not Found' });
+    expect(res.status).toBe(500);
+    expect(res.body).toEqual({ error: 'Internal server error' });
+  });
+
+  // The 404 handler records the caller as suspicious. A fault on this side is not the
+  // caller misbehaving, and letting it land there put server bugs into the signal the
+  // anomaly views read.
+  it('does not record a server error as suspicious activity', async () => {
+    await request(built).get('/__test_plain_error');
+
+    expect(AuthEventService.requestSuspicious).not.toHaveBeenCalled();
   });
 
   it('falls back to a 500 when the error pipeline itself throws', async () => {