fix: resolve traceroute race condition for fast-responding nodes (#2364)#2366
fix: resolve traceroute race condition for fast-responding nodes (#2364)#2366
Conversation
recordTracerouteRequest() for PostgreSQL/MySQL fired an un-awaited async IIFE to insert the pending traceroute record. For nearby nodes, the response arrived before the DB insert completed, causing "No response received" in the UI despite the response being in logs/packet monitor. Fix: make recordTracerouteRequest() async and properly await the DB operations, ensuring the pending record is written before the response can be processed. Closes #2364 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| } | ||
|
|
||
| recordTracerouteRequest(fromNodeNum: number, toNodeNum: number): void { | ||
| async recordTracerouteRequest(fromNodeNum: number, toNodeNum: number): Promise<void> { |
There was a problem hiding this comment.
✅ Great fix! Making this method properly async resolves the race condition. The previous fire-and-forget IIFE meant fast traceroute responses could arrive before the pending record was inserted into the database.
Consider adding a brief comment here explaining why the await is critical:
| async recordTracerouteRequest(fromNodeNum: number, toNodeNum: number): Promise<void> { | |
| async recordTracerouteRequest(fromNodeNum: number, toNodeNum: number): Promise<void> { | |
| // NOTE: This MUST be awaited to prevent race conditions with fast-responding nodes | |
| const now = Date.now(); |
| // For PostgreSQL/MySQL, use async repository | ||
| if (this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') { |
There was a problem hiding this comment.
✅ Perfect removal of the problematic pattern. The fire-and-forget IIFE was the root cause of the race condition. Now the database operations are properly awaited before the method returns, ensuring the pending traceroute record exists when fast responses arrive.
| // Group 4: Traceroutes | ||
| async recordTracerouteRequestAsync(fromNodeNum: number, toNodeNum: number): Promise<void> { | ||
| this.recordTracerouteRequest(fromNodeNum, toNodeNum); | ||
| await this.recordTracerouteRequest(fromNodeNum, toNodeNum); |
There was a problem hiding this comment.
✅ Critical fix! This change from fire-and-forget to properly awaited call is essential. The async wrapper pattern throughout the codebase expects the inner method to be awaited, and this was the missing piece causing the race condition.
This now properly aligns with the multi-database architecture requirement that "ALL database methods in DatabaseService must be async".
Summary
Fixes #2364 — traceroutes showed "No response received" for nearby/fast-responding nodes despite the response being visible in logs and packet monitor.
Root Cause
recordTracerouteRequest()for PostgreSQL/MySQL fired an un-awaited async IIFE to insert the pending traceroute record. For nearby nodes, the traceroute response arrived before the DB insert completed:sendTraceroute()→ sends packet, callsawait recordTracerouteRequestAsync()processTracerouteMessage()runsFix
Made
recordTracerouteRequest()properly async — removed the fire-and-forget IIFE and directly awaits the DB operations. The async wrapper now correctly awaits the inner method.Files Changed
src/services/database.tsrecordTracerouteRequestasync, remove un-awaited IIFE, await in async wrapperTest plan
🤖 Generated with Claude Code