POST /notify/🆔 standardize response shape — always include stored field
Bug: when the event was not found at the relay, the handler returned
{ ok: true, skipped: true }, which did not match the documented contract
{ ok: true, stored: boolean } in README.md.
Fix: change the 'skipped' response to { ok: true, stored: false }, so the
response shape is consistent across all code paths.
- src/server.ts: changed line 287 from { ok: true, skipped: true } to
{ ok: true, stored: false }, with updated comment
- README.md: added a note documenting the skip case (stored: false)
- test/notify-response-shape.test.ts: new test that asserts stored=false
and that skipped is never present
This commit is contained in:
parent
a4bc587d64
commit
b54fb4f891
3 changed files with 84 additions and 2 deletions
|
|
@ -85,6 +85,10 @@ Response: { ok: true, stored: boolean }
|
|||
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=...
|
||||
Confirm email address via link from confirmation email.
|
||||
|
||||
|
|
|
|||
|
|
@ -285,8 +285,8 @@ addRoute('post', '/notify/:id', async (req: Request, res: Response) => {
|
|||
storedEvent = fetched
|
||||
|
||||
if (!storedEvent) {
|
||||
// Event not found at relay — don't 404, just skip
|
||||
return res.json({ ok: true, skipped: true })
|
||||
// Event not found at relay — don't 404, reflect that nothing was stored
|
||||
return res.json({ ok: true, stored: false })
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
78
test/notify-response-shape.test.ts
Normal file
78
test/notify-response-shape.test.ts
Normal 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)
|
||||
})
|
||||
})
|
||||
Loading…
Reference in a new issue