diff --git a/.changeset/fix-next-navigation-signal-phantom-error.md b/.changeset/fix-next-navigation-signal-phantom-error.md new file mode 100644 index 00000000..a866201c --- /dev/null +++ b/.changeset/fix-next-navigation-signal-phantom-error.md @@ -0,0 +1,5 @@ +--- +"evlog": patch +--- + +Fix `withEvlog` (Next.js) logging a phantom ERROR at status 500 for every `redirect()`/`notFound()`/`forbidden()`/`unauthorized()` call. These APIs throw an internal Next.js control-flow signal that isn't a real error — `withEvlog` now detects it via `unstable_rethrow` and rethrows it untouched instead of logging and emitting an error event. diff --git a/packages/evlog/src/next/handler.ts b/packages/evlog/src/next/handler.ts index df074bde..499e02b9 100644 --- a/packages/evlog/src/next/handler.ts +++ b/packages/evlog/src/next/handler.ts @@ -117,6 +117,34 @@ async function callEnrichAndDrain( run().catch(() => {}) } +/** + * Rethrow `error` if Next.js recognizes it as an internal control-flow signal + * (`redirect()`, `notFound()`, `forbidden()`, `unauthorized()`, ...) rather than a real + * application error. No-ops otherwise, letting the caller continue with normal error + * logging. + * + * Delegates to Next's own `unstable_rethrow`, which covers current and future signal + * types without hardcoding digest strings — including signals wrapped in `error.cause`, + * which it rethrows unwrapped. `next/navigation` import failures (older Next.js versions, + * non-Next runtimes) are swallowed so the caller falls through to normal logging. + */ +async function rethrowIfNextNavigationSignal(error: unknown): Promise { + let unstableRethrow: ((error: unknown) => void) | undefined + try { + const nextNavigation = await import('next/navigation') as { + unstable_rethrow?: (error: unknown) => void + } + unstableRethrow = nextNavigation.unstable_rethrow + } catch { + return + } + + if (typeof unstableRethrow !== 'function') return + + // Throws (possibly the unwrapped `error.cause`) iff this is a recognized signal. + unstableRethrow(error) +} + async function emitRequestEvent( logger: ReturnType, requestInfo: { method: string, path: string, requestId: string }, @@ -254,6 +282,11 @@ export function createWithEvlog(options: NextEvlogOptions) { return result as Awaited } catch (error) { + // redirect()/notFound()/forbidden()/unauthorized() throw a control-flow signal + // that Next.js turns into a real response — not an application error. Rethrow it + // untouched instead of logging a phantom ERROR@500 (see #436). + await rethrowIfNextNavigationSignal(error) + const err = error instanceof Error ? error : new Error(String(error)) await enrichNextErrorStackForDev(err, { pretty: state.options.pretty }) logger.error(err) diff --git a/packages/evlog/test/next/handler.test.ts b/packages/evlog/test/next/handler.test.ts index 87100b43..3bf9f1a6 100644 --- a/packages/evlog/test/next/handler.test.ts +++ b/packages/evlog/test/next/handler.test.ts @@ -131,6 +131,122 @@ describe('withEvlog', () => { expect(parsed.status).toBe(404) }) + it('rethrows redirect() navigation signals without logging a phantom error (#436)', async () => { + const drainMock = vi.fn() + const withEvlog = createWithEvlog({ pretty: false, drain: drainMock }) + + const redirectError = Object.assign(new Error('NEXT_REDIRECT'), { + digest: 'NEXT_REDIRECT;replace;/foo;307;', + }) + const handler = withEvlog((_: Request) => { + throw redirectError + }) + + const request = new Request('http://localhost/go', { method: 'GET' }) + await expect(handler(request)).rejects.toBe(redirectError) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect(consoleSpy).not.toHaveBeenCalled() + expect(drainMock).not.toHaveBeenCalled() + }) + + it('rethrows notFound() navigation signals without logging a phantom error (#436)', async () => { + const drainMock = vi.fn() + const withEvlog = createWithEvlog({ pretty: false, drain: drainMock }) + + const notFoundError = Object.assign(new Error('NEXT_HTTP_ERROR_FALLBACK'), { + digest: 'NEXT_HTTP_ERROR_FALLBACK;404', + }) + const handler = withEvlog((_: Request) => { + throw notFoundError + }) + + const request = new Request('http://localhost/missing', { method: 'GET' }) + await expect(handler(request)).rejects.toBe(notFoundError) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect(consoleSpy).not.toHaveBeenCalled() + expect(drainMock).not.toHaveBeenCalled() + }) + + it('rethrows forbidden() navigation signals without logging a phantom error (#436)', async () => { + const drainMock = vi.fn() + const withEvlog = createWithEvlog({ pretty: false, drain: drainMock }) + + const forbiddenError = Object.assign(new Error('NEXT_HTTP_ERROR_FALLBACK'), { + digest: 'NEXT_HTTP_ERROR_FALLBACK;403', + }) + const handler = withEvlog((_: Request) => { + throw forbiddenError + }) + + const request = new Request('http://localhost/admin', { method: 'GET' }) + await expect(handler(request)).rejects.toBe(forbiddenError) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect(consoleSpy).not.toHaveBeenCalled() + expect(drainMock).not.toHaveBeenCalled() + }) + + it('rethrows unauthorized() navigation signals without logging a phantom error (#436)', async () => { + const drainMock = vi.fn() + const withEvlog = createWithEvlog({ pretty: false, drain: drainMock }) + + const unauthorizedError = Object.assign(new Error('NEXT_HTTP_ERROR_FALLBACK'), { + digest: 'NEXT_HTTP_ERROR_FALLBACK;401', + }) + const handler = withEvlog((_: Request) => { + throw unauthorizedError + }) + + const request = new Request('http://localhost/account', { method: 'GET' }) + await expect(handler(request)).rejects.toBe(unauthorizedError) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect(consoleSpy).not.toHaveBeenCalled() + expect(drainMock).not.toHaveBeenCalled() + }) + + it('rethrows a navigation signal wrapped in error.cause without logging (#436)', async () => { + const drainMock = vi.fn() + const withEvlog = createWithEvlog({ pretty: false, drain: drainMock }) + + const redirectError = Object.assign(new Error('NEXT_REDIRECT'), { + digest: 'NEXT_REDIRECT;replace;/foo;307;', + }) + // Some frameworks/middleware wrap the original error — unstable_rethrow unwraps + // `.cause` and rethrows the inner signal, not the outer wrapper. + const wrapper = new Error('wrapped', { cause: redirectError }) + const handler = withEvlog((_: Request) => { + throw wrapper + }) + + const request = new Request('http://localhost/go', { method: 'GET' }) + await expect(handler(request)).rejects.toBe(redirectError) + + expect(consoleErrorSpy).not.toHaveBeenCalled() + expect(consoleSpy).not.toHaveBeenCalled() + expect(drainMock).not.toHaveBeenCalled() + }) + + it('still logs a real error whose message happens to mention NEXT_REDIRECT (#436)', async () => { + const withEvlog = createWithEvlog({ pretty: false }) + + const handler = withEvlog((_: Request) => { + // No `digest` set — unstable_rethrow must not treat this as a navigation signal. + throw new Error('something about NEXT_REDIRECT went wrong') + }) + + const request = new Request('http://localhost/api/test', { method: 'GET' }) + await expect(handler(request)).rejects.toThrow('something about NEXT_REDIRECT went wrong') + + expect(consoleErrorSpy).toHaveBeenCalled() + const [[output]] = consoleErrorSpy.mock.calls + const parsed = JSON.parse(output) + expect(parsed.level).toBe('error') + expect(parsed.status).toBe(500) + }) + it('calls drain callback with emitted event', async () => { const drainMock = vi.fn() const withEvlog = createWithEvlog({ pretty: false, drain: drainMock })