An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.
fix(relay): address PR review issues for MM-86
- Remove `&& attempt < 2` guard: unique violations now always retry;
post-loop error becomes the exhaustion case (was dead code before)
- Fix comment: "Retry up to 3 times" → "Attempt up to 3 times total (2 retries)"
- Add expires_at window assertion to persistence tests (5s tolerance)
- Add non_unique_db_error_returns_500_without_retry test (closes pool before request)
- Annotate begin/commit in insert_claim_codes with inspect_err logging
- Log non-UTF-8 Authorization header at debug level
- Add doc comment to ClaimCodesResponse.codes field
authored by
Tangled
dde425b4
6dac75c1
+66
-15
+66
-15
crates/relay/src/routes/claim_codes.rs
+66
-15
crates/relay/src/routes/claim_codes.rs
···
31
31
32
32
#[derive(Serialize)]
33
33
pub struct ClaimCodesResponse {
34
+
/// 6-character uppercase alphanumeric strings, unique within this batch.
34
35
codes: Vec<String>,
35
36
}
36
37
···
49
50
50
51
let auth_value = headers
51
52
.get(axum::http::header::AUTHORIZATION)
52
-
.and_then(|v| v.to_str().ok())
53
+
.and_then(|v| {
54
+
v.to_str()
55
+
.inspect_err(|_| {
56
+
tracing::debug!(
57
+
"Authorization header contains non-UTF-8 bytes; treating as absent"
58
+
);
59
+
})
60
+
.ok()
61
+
})
53
62
.unwrap_or("");
54
63
55
64
let provided_token = auth_value.strip_prefix("Bearer ").ok_or_else(|| {
···
86
95
}
87
96
88
97
// --- Generate unique codes and insert in a single transaction ---
89
-
// Retry up to 3 times on the rare event of a uniqueness conflict with an
90
-
// existing DB row (probability ≈ existing_codes / 36^6 per code generated).
98
+
// Attempt up to 3 times total (2 retries) on the rare event of a uniqueness
99
+
// conflict with an existing DB row (probability ≈ existing_codes / 36^6 per code).
91
100
for attempt in 0..3_usize {
92
101
let codes = generate_unique_codes(payload.count as usize);
93
102
match insert_claim_codes(&state.db, &codes, payload.expires_in_hours).await {
94
103
Ok(()) => return Ok(Json(ClaimCodesResponse { codes })),
95
-
Err(e) if is_unique_violation(&e) && attempt < 2 => {
104
+
Err(e) if is_unique_violation(&e) => {
96
105
tracing::warn!(attempt, "claim code uniqueness conflict; retrying");
97
106
continue;
98
107
}
···
137
146
expires_in_hours: u32,
138
147
) -> Result<(), sqlx::Error> {
139
148
let offset = format!("+{expires_in_hours} hours");
140
-
let mut tx = db.begin().await?;
149
+
let mut tx = db.begin().await.inspect_err(|e| {
150
+
tracing::error!(error = %e, "failed to begin claim_codes transaction");
151
+
})?;
141
152
for code in codes {
142
153
sqlx::query(
143
154
"INSERT INTO claim_codes (code, expires_at, created_at) \
···
148
159
.execute(&mut *tx)
149
160
.await?;
150
161
}
151
-
tx.commit().await?;
162
+
tx.commit().await.inspect_err(|e| {
163
+
tracing::error!(error = %e, "failed to commit claim_codes transaction");
164
+
})?;
152
165
Ok(())
153
166
}
154
167
···
257
270
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
258
271
let code = json["codes"][0].as_str().unwrap();
259
272
260
-
// expires_at should be roughly 24h from now; verify it is in the future.
261
273
let expires_at: String =
262
274
sqlx::query_scalar("SELECT expires_at FROM claim_codes WHERE code = ?")
263
275
.bind(code)
···
265
277
.await
266
278
.unwrap();
267
279
268
-
// SQLite datetime('now', '+24 hours') produces a value > datetime('now').
269
-
let is_future: bool = sqlx::query_scalar("SELECT ? > datetime('now')")
270
-
.bind(&expires_at)
271
-
.fetch_one(&db)
272
-
.await
273
-
.unwrap();
274
-
assert!(is_future, "expires_at must be in the future");
280
+
// Verify expires_at is within 5 seconds of 24h from now.
281
+
let within_window: bool = sqlx::query_scalar(
282
+
"SELECT ABS(strftime('%s', ?) - strftime('%s', datetime('now', '+24 hours'))) < 5",
283
+
)
284
+
.bind(&expires_at)
285
+
.fetch_one(&db)
286
+
.await
287
+
.unwrap();
288
+
assert!(
289
+
within_window,
290
+
"expires_at must be approximately 24h from now"
291
+
);
275
292
}
276
293
277
294
// ── Code format ───────────────────────────────────────────────────────────
···
335
352
336
353
#[tokio::test]
337
354
async fn codes_persisted_in_db_with_pending_status() {
338
-
// MM-86.AC3.1: stored with redeemed_at NULL (pending)
355
+
// MM-86.AC3.1: stored with redeemed_at NULL (pending) and correct expiry
339
356
let state = test_state_with_admin_token().await;
340
357
let db = state.db.clone();
341
358
···
366
383
row.1.is_none(),
367
384
"redeemed_at must be NULL for a freshly generated code"
368
385
);
386
+
387
+
// expires_at must be approximately 48h from now (within 5 seconds).
388
+
let within_window: bool = sqlx::query_scalar(
389
+
"SELECT ABS(strftime('%s', ?) - strftime('%s', datetime('now', '+48 hours'))) < 5",
390
+
)
391
+
.bind(&row.0)
392
+
.fetch_one(&db)
393
+
.await
394
+
.unwrap();
395
+
assert!(
396
+
within_window,
397
+
"expires_at must be approximately 48h from now"
398
+
);
369
399
}
400
+
}
401
+
402
+
// ── Retry / DB error paths ────────────────────────────────────────────────
403
+
404
+
#[tokio::test]
405
+
async fn non_unique_db_error_returns_500_without_retry() {
406
+
// Closing the pool before the request causes db.begin() to fail with a
407
+
// non-unique-violation error. The handler must return 500 immediately
408
+
// (no retry) and must not panic.
409
+
let state = test_state_with_admin_token().await;
410
+
state.db.close().await;
411
+
412
+
let response = app(state)
413
+
.oneshot(post_claim_codes(
414
+
r#"{"count": 1, "expiresInHours": 24}"#,
415
+
Some("test-admin-token"),
416
+
))
417
+
.await
418
+
.unwrap();
419
+
420
+
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
370
421
}
371
422
372
423
// ── Input validation ──────────────────────────────────────────────────────