Standardize POST /notify/:id response shape — always include stored field #13

Merged
matt merged 1 commit from mailship-nl0-post-notify-response-shape-ok-skipped-la-387 into main 2026-09-14 20:00:32 +00:00
3 changed files with 84 additions and 2 deletions

View file

@ -85,6 +85,10 @@ Response: { ok: true, stored: boolean }
Returns 404 if subscription not found or inactive. Returns 404 if subscription not found or inactive.
``` ```
When the event is not found at the relay (e.g. it was deleted or never arrived),
the endpoint returns `{ ok: true, stored: false }` — the event is silently skipped
rather than erroring. The `stored` field is always present in a 200 response.
### GET /confirm?token=... ### GET /confirm?token=...
Confirm email address via link from confirmation email. Confirm email address via link from confirmation email.

View file

@ -285,8 +285,8 @@ addRoute('post', '/notify/:id', async (req: Request, res: Response) => {
storedEvent = fetched storedEvent = fetched
if (!storedEvent) { if (!storedEvent) {
// Event not found at relay — don't 404, just skip // Event not found at relay — don't 404, reflect that nothing was stored
return res.json({ ok: true, skipped: true }) return res.json({ ok: true, stored: false })
} }
} }

View file

@ -0,0 +1,78 @@
// POST /notify/:id response shape test
//
// Verifies that the endpoint always includes a `stored` boolean
// in its response, matching the documented contract in README.md:
// Response: { ok: true, stored: boolean }
//
// Bug: when the event is not found at the relay, the handler returns
// { ok: true, skipped: true }
// missing the documented `stored` field.
import { describe, it, expect, beforeAll, afterAll } from 'vitest'
import * as db from '../src/database.js'
import { server } from '../src/server.js'
import { createServer, type Server } from 'http'
// Partially mock @welshman/net so that `load()` returns an empty array,
// simulating the case where the relay does not have the requested event,
// while preserving all other exports that other modules depend on.
vi.mock('@welshman/net', async (importOriginal) => {
const actual = await importOriginal()
return {
...(actual as Record<string, unknown>),
load: vi.fn().mockResolvedValue([]),
}
})
describe('notify_response_shape', () => {
let httpServer: Server
let baseUrl: string
let subId: string
beforeAll(async () => {
await db.migrate()
// Create and confirm a subscription we can use for the notify call
const pubkey = 'shape-test-pk-' + Date.now()
const email = 'shape-test-' + Date.now() + '@example.com'
const sub = await db.insertSubscription(pubkey, email, 'daily')
const confirmed = await db.confirmSubscription(sub.key)
subId = confirmed.id
// Start the express server on a random available port
await new Promise<void>((resolve) => {
httpServer = createServer(server)
httpServer.listen(0, () => {
const addr = httpServer.address()
if (addr && typeof addr === 'object') {
baseUrl = `http://localhost:${addr.port}`
}
resolve()
})
})
})
afterAll(async () => {
httpServer?.close()
})
it('returns stored=false instead of skipped=true when event not found', async () => {
const res = await fetch(`${baseUrl}/notify/${subId}`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
id: 'nonexistent-' + Date.now(),
relay: 'wss://relay.damus.io',
}),
})
const body = await res.json()
// The documented contract says: { ok: true, stored: boolean }
// The current buggy code returns: { ok: true, skipped: true }
expect(body).not.toHaveProperty('skipped')
expect(body).toHaveProperty('stored')
expect(body.stored).toBe(false)
expect(body.ok).toBe(true)
})
})