nudger: the due query was invalid sql, and failed silently every tick
`dueNudges` passed a JS Date as a parameter on the left of an interval subtraction. postgres will not infer a type there and refused the whole statement, so the tick threw once a minute and no nudge could ever fire. the symptom was the worst kind: the row sat in the table enabled, sent_count 0, last_fired_at null — indistinguishable from a schedule that was armed and simply had not come due yet. it was found by checking the row after arming it rather than by trusting the log line that said "armed". `now()` in sql instead, which also means the comparison uses the database's clock rather than the platform's. the test asserts only that the query EXECUTES. what broke was the sql being invalid, and no amount of reasoning about interval arithmetic would have caught that — only running it against a real postgres does.
This commit is contained in:
@@ -0,0 +1,14 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { dueNudges } from './queries';
|
||||
|
||||
describe('dueNudges', () => {
|
||||
// It failed on EVERY tick and the only symptom was a log line nobody was reading: the row sat there
|
||||
// enabled, with sent_count 0, looking perfectly armed. A JS Date bound as a parameter on the left of an
|
||||
// interval subtraction is a type Postgres will not infer, so it refused the whole statement.
|
||||
//
|
||||
// Asserting only that it EXECUTES. The due-ness logic is Postgres arithmetic; what broke was the SQL
|
||||
// being invalid, which no amount of reasoning about intervals would have caught.
|
||||
test('is valid SQL against the real database', async () => {
|
||||
expect(Array.isArray(await dueNudges())).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -50,7 +50,7 @@ export async function cancelNudge(userId: number, sessionId: string): Promise<vo
|
||||
* ago. Whether the session is actually IDLE is not asked here — that lives in the sidecar, not the
|
||||
* database, and the scheduler checks it immediately before speaking.
|
||||
*/
|
||||
export async function dueNudges(now: Date = new Date()): Promise<NudgeSelect[]> {
|
||||
export async function dueNudges(): Promise<NudgeSelect[]> {
|
||||
return db
|
||||
.select()
|
||||
.from(nudges)
|
||||
@@ -60,7 +60,10 @@ export async function dueNudges(now: Date = new Date()): Promise<NudgeSelect[]>
|
||||
lt(nudges.sentCount, nudges.maxCount),
|
||||
or(
|
||||
isNull(nudges.lastFiredAt),
|
||||
lt(nudges.lastFiredAt, sql`${now} - (${nudges.everySeconds} * interval '1 second')`),
|
||||
// `now()` in SQL rather than a JS Date bound as a parameter. Postgres cannot infer the type of
|
||||
// a parameter on the left of an interval subtraction and refuses the whole statement — which
|
||||
// failed EVERY tick, silently, with the row sitting there looking armed.
|
||||
lt(nudges.lastFiredAt, sql`now() - (${nudges.everySeconds} * interval '1 second')`),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user