Files
zpan/server/middleware/logger.test.ts
T
Jasper Van 8abca2f88c refactor(errors)!: unify error handling on typed AppError + single jsonError renderer (#445)
Collapse the two error conventions (string-reason `{ok:false,reason}` outcomes
and thrown domain-error classes) onto one. Usecases now produce typed `AppError`
values via factories (`notFound()`/`quotaExceeded()`/`featureBlocked()`/…);
handlers `throw result.error`; and `jsonError` (renamed from `renderError`) is the
single place that renders any error to an AIP-193 body + access-log line, in
`app.onError`/accessLog.

Why: the previous setup had a string→code mapping (`outcomeError` + the `OUTCOME`
table) living in parallel with a type→code mapping (`mapDomainError`), plus inline
`apiError(c, <status>, …)` calls that hand-wrote the status at every site — exactly
the drift that left the same `quota_exceeded` at 400 in one handler and 422 in the
rest. Now the status/reason live once, in the factory.

- Add `server/usecases/ports/app-error.ts`: `AppError` + factories. Status/reason
  are baked in per factory, so no usecase or handler writes an HTTP code or a
  magic-string reason. `AppError` also carries optional response headers
  (`Retry-After`) via a `rateLimited()` factory.
- Delete `apiError`, `outcomeError`, the `OUTCOME` table, and the dead `ApiError`
  class. The 67 inline guard/middleware `apiError` sites became `throw <factory>()`.
- Control-flow outcomes a handler branches on (not just renders) stay discriminated
  reasons (e.g. `deleteObject` `not_trashed`); internal shared sub-usecases
  (traffic-metering, licensing internals) keep string reasons, mapped at the boundary.
- Regenerate the Go OpenAPI client (saveShare gained a 422 response).

BREAKING CHANGE: POST /shares/{token}/objects quota rejection now returns 422
(was an inconsistent 400); every other quota path already returned 422.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-17 01:47:59 -04:00

103 lines
3.5 KiB
TypeScript

import type { Handler } from 'hono'
import { Hono } from 'hono'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { insufficientCredits, NameConflictError, notFound } from '../usecases/ports'
import { jsonError } from './error-handler'
import { accessLog } from './logger'
import type { Env } from './platform'
// Parse one `key="json"` access-log line into a record.
function parseLine(line: string): Record<string, string> {
const out: Record<string, string> = {}
for (const m of line.matchAll(/(\w+)=("(?:[^"\\]|\\.)*"|\S+)/g)) {
out[m[1]] = m[2].startsWith('"') ? (JSON.parse(m[2]) as string) : m[2]
}
return out
}
describe('accessLog', () => {
let lines: string[]
beforeEach(() => {
lines = []
vi.spyOn(console, 'log').mockImplementation((line: string) => {
lines.push(line)
})
})
afterEach(() => vi.restoreAllMocks())
// Mirror production: accessLog at the boundary, errorLog initialised like
// platformMiddleware, and app.onError rendering thrown errors via jsonError
// (Hono routes throws there, not to a middleware catch — see app.ts).
function appWith(handler: Handler<Env>) {
const app = new Hono<Env>()
app.use('*', accessLog)
app.use('*', async (c, next) => {
c.set('errorLog', null)
await next()
})
app.get('/x', handler)
app.onError((err, c) => jsonError(c, err))
return app
}
it('logs a success without an error field', async () => {
const app = appWith((c) => c.json({ ok: true }, 200))
await app.request('/x')
const f = parseLine(lines[0])
expect(f.status).toBe('200')
expect(f.error).toBeUndefined()
expect(f.reason).toBeUndefined()
})
it('logs reason + message for a thrown AppError', async () => {
const app = appWith(() => {
throw notFound('Widget not found')
})
const res = await app.request('/x')
expect(res.status).toBe(404)
const f = parseLine(lines[0])
expect(f.status).toBe('404')
expect(f.reason).toBe('NOT_FOUND')
expect(f.error).toBe('Widget not found')
})
it('carries the specific reason + metadata message for a special error', async () => {
const app = appWith(() => {
throw insufficientCredits('Insufficient credits', { metadata: { resource: 'storage_egress' } })
})
await app.request('/x')
const f = parseLine(lines[0])
expect(f.reason).toBe('INSUFFICIENT_CREDITS')
expect(f.error).toBe('Insufficient credits')
})
it('logs a thrown domain error with its MAPPED status, not 500', async () => {
const app = appWith(() => {
throw new NameConflictError('doc.txt', 'id-1')
})
const res = await app.request('/x')
expect(res.status).toBe(409)
const f = parseLine(lines[0])
expect(f.status).toBe('409')
expect(f.reason).toBe('NAME_CONFLICT')
})
it('logs the full cause chain for an unhandled 500 (and hides it from the client)', async () => {
const app = appWith(() => {
const err = new Error('top') as Error & { cause?: unknown }
err.cause = new Error('D1_ERROR: disk full')
throw err
})
const res = await app.request('/x')
expect(res.status).toBe(500)
// Client body is generic — no internal detail leaks.
expect(((await res.json()) as { error: { message: string } }).error.message).toBe('Internal Server Error')
// The access log keeps the full chain.
const f = parseLine(lines[0])
expect(f.status).toBe('500')
expect(f.reason).toBe('INTERNAL')
expect(f.error).toContain('top')
expect(f.error).toContain('D1_ERROR: disk full')
})
})