refactor(invites): typed InviteUnavailableReason + real rollback test
Quality-review polish on Task 8: 1. InviteUnavailableError gains a typed public readonly `reason` field (the union 'not found' | 'revoked' | 'expired' | 'exhausted'). Task 11 route handler can switch on the discriminant to produce user-facing copy without parsing the message string. 2. The original "aborts transaction if insertUser throws" test was vacuous — the callback threw before any DB write, so SQLite ROLLBACK never fired and the post-conditions were trivially true. Replaced with two tests: one that explicitly validates the synchronous short-circuit (no DB work happens at all), and a second that writes a real users row inside the callback then throws AFTER the write, proving the SQLite ROLLBACK actually reverts the in-callback write.
This commit is contained in:
@@ -531,13 +531,13 @@ describe('redeemInvite', () => {
|
|||||||
expect(redemptions).toHaveLength(0);
|
expect(redemptions).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('aborts transaction (no usedCount increment, no redemption row) if insertUser throws', () => {
|
it('does not bump usedCount or write a redemption when insertUser throws synchronously', () => {
|
||||||
const adminId = seedAdmin();
|
const adminId = seedAdmin();
|
||||||
const inv = createInvite({ name: 'a', maxUses: 5, expiresAt: null }, adminId);
|
const inv = createInvite({ name: 'a', maxUses: 5, expiresAt: null }, adminId);
|
||||||
|
|
||||||
expect(() => redeemInvite(inv.token, () => {
|
expect(() => redeemInvite(inv.token, () => {
|
||||||
throw new Error('username collision');
|
throw new Error('username taken');
|
||||||
})).toThrow('username collision');
|
})).toThrow('username taken');
|
||||||
|
|
||||||
// usedCount unchanged
|
// usedCount unchanged
|
||||||
const row = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, inv.id)).get();
|
const row = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, inv.id)).get();
|
||||||
@@ -548,6 +548,34 @@ describe('redeemInvite', () => {
|
|||||||
expect(redemptions).toHaveLength(0);
|
expect(redemptions).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rolls back insertUser writes when a later step throws (true SQLite ROLLBACK)', () => {
|
||||||
|
const adminId = seedAdmin();
|
||||||
|
const inv = createInvite({ name: 'a', maxUses: 5, expiresAt: null }, adminId);
|
||||||
|
const newUserId = 'rollback-user-1';
|
||||||
|
|
||||||
|
// The callback writes a real users row, then throws AFTER the write.
|
||||||
|
// If the txn truly rolls back, the users row must not exist after the call.
|
||||||
|
expect(() => redeemInvite(inv.token, () => {
|
||||||
|
testDb.insert(schema.users).values({
|
||||||
|
id: newUserId,
|
||||||
|
username: 'will-be-rolled-back',
|
||||||
|
passwordHash: 'x',
|
||||||
|
createdAt: Date.now(),
|
||||||
|
}).run();
|
||||||
|
throw new Error('post-insert failure');
|
||||||
|
})).toThrow('post-insert failure');
|
||||||
|
|
||||||
|
// Proves SQLite ROLLBACK reverted the user insert AND the would-be usedCount bump
|
||||||
|
const u = testDb.select().from(schema.users).where(eq(schema.users.id, newUserId)).get();
|
||||||
|
expect(u).toBeUndefined();
|
||||||
|
|
||||||
|
const after = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, inv.id)).get();
|
||||||
|
expect(after?.usedCount).toBe(0);
|
||||||
|
|
||||||
|
const redemptions = testDb.select().from(schema.inviteRedemptions).where(eq(schema.inviteRedemptions.inviteId, inv.id)).all();
|
||||||
|
expect(redemptions).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
it('throws InviteUnavailableError when token not found', () => {
|
it('throws InviteUnavailableError when token not found', () => {
|
||||||
expect(() => redeemInvite('aaaaaaaaaaaaaaaaaaaaaa', () => ({ id: 'x', username: 'x' }))).toThrow(InviteUnavailableError);
|
expect(() => redeemInvite('aaaaaaaaaaaaaaaaaaaaaa', () => ({ id: 'x', username: 'x' }))).toThrow(InviteUnavailableError);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -468,14 +468,23 @@ export function reinstateInvite(id: string, req: ReinstateInviteRequest): Reinst
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Discriminant union of reasons an invite cannot be redeemed. Surfaced as a
|
||||||
|
* typed public field on `InviteUnavailableError` so the HTTP register route
|
||||||
|
* can switch on it to produce user-facing copy without parsing the message
|
||||||
|
* string. Mirrors the non-active subset of `InviteStatus` plus `'not found'`
|
||||||
|
* for the missing-token case.
|
||||||
|
*/
|
||||||
|
export type InviteUnavailableReason = 'not found' | 'revoked' | 'expired' | 'exhausted';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Thrown when an invite cannot be redeemed because its current state forbids
|
* Thrown when an invite cannot be redeemed because its current state forbids
|
||||||
* it (token not found, revoked, expired, exhausted). Caller (HTTP register
|
* it (token not found, revoked, expired, exhausted). Caller (HTTP register
|
||||||
* route) maps this to 403 Forbidden. The reason string is preserved in the
|
* route) maps this to 403 Forbidden. The `reason` field is the structured
|
||||||
* message for debugging — the user-facing copy is constructed by the route.
|
* discriminant; the message string is preserved for debugging/logging.
|
||||||
*/
|
*/
|
||||||
export class InviteUnavailableError extends Error {
|
export class InviteUnavailableError extends Error {
|
||||||
constructor(reason: string) {
|
constructor(public readonly reason: InviteUnavailableReason) {
|
||||||
super(`Invite unavailable: ${reason}`);
|
super(`Invite unavailable: ${reason}`);
|
||||||
this.name = 'InviteUnavailableError';
|
this.name = 'InviteUnavailableError';
|
||||||
}
|
}
|
||||||
@@ -513,6 +522,9 @@ export function redeemInvite(
|
|||||||
if (!row) throw new InviteUnavailableError('not found');
|
if (!row) throw new InviteUnavailableError('not found');
|
||||||
const status = inviteStatus(row);
|
const status = inviteStatus(row);
|
||||||
if (status !== 'active') {
|
if (status !== 'active') {
|
||||||
|
// 'active' is excluded by the guard above, so `status` is necessarily
|
||||||
|
// one of 'revoked' | 'expired' | 'exhausted' — all valid
|
||||||
|
// InviteUnavailableReason values. TS narrows the union here.
|
||||||
throw new InviteUnavailableError(status);
|
throw new InviteUnavailableError(status);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user