Add TTL-based eviction for orphaned initialHostByRequest entries#46
Add TTL-based eviction for orphaned initialHostByRequest entries#46
initialHostByRequest entries#46Conversation
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
Co-authored-by: ormidales <46538211+ormidales@users.noreply.github.com>
initialHostByRequest entries
There was a problem hiding this comment.
Pull request overview
This PR addresses orphaned entries in the initialHostByRequest tracking map that accumulate when webRequest lifecycle events don't fire (e.g., when requests are canceled by other extensions). The solution introduces a TTL-based cleanup mechanism alongside existing eager deletion paths.
Changes:
- Migrated
initialHostByRequestfrom storing raw hostname strings to structured objects containing{ host, trackedAt } - Added periodic TTL-based sweeper (60-second interval) to remove entries older than
REQUEST_TRACK_TTL_MS - Updated retrieval logic in
onHeadersReceivedwith backward compatibility for both object and legacy string formats
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }; | ||
|
|
||
| setInterval(cleanupStaleTrackedRequests, REQUEST_TRACK_TTL_MS); |
There was a problem hiding this comment.
The setInterval timer created here will run indefinitely without a cleanup mechanism. While this is acceptable for a background script that lives for the extension's lifetime, consider storing the interval ID if you need to clear it during extension unload or testing: const cleanupIntervalId = setInterval(cleanupStaleTrackedRequests, REQUEST_TRACK_TTL_MS);
| setInterval(cleanupStaleTrackedRequests, REQUEST_TRACK_TTL_MS); | |
| const cleanupIntervalId = setInterval(cleanupStaleTrackedRequests, REQUEST_TRACK_TTL_MS); |
| @@ -1,5 +1,6 @@ | |||
| const initialHostByRequest = new Map(); | |||
| const MAX_TRACKED_REQUESTS = 1000; | |||
| const REQUEST_TRACK_TTL_MS = 60 * 1000; | |||
There was a problem hiding this comment.
The 60-second TTL may be too short for slow network conditions or complex redirect chains. Consider increasing to 2-5 minutes (120000-300000 ms) to reduce the risk of prematurely evicting legitimate long-running requests while still preventing indefinite accumulation.
| const REQUEST_TRACK_TTL_MS = 60 * 1000; | |
| const REQUEST_TRACK_TTL_MS = 5 * 60 * 1000; |
|
|
||
| const cleanupStaleTrackedRequests = (now = Date.now()) => { | ||
| for (const [requestId, trackedRequest] of initialHostByRequest.entries()) { | ||
| if (trackedRequest && typeof trackedRequest === "object" && now - trackedRequest.trackedAt > REQUEST_TRACK_TTL_MS) { |
There was a problem hiding this comment.
The condition relies on NaN > REQUEST_TRACK_TTL_MS being false when trackedAt is missing. Consider adding an explicit check for trackedAt to make the logic clearer: if (trackedRequest && typeof trackedRequest === "object" && typeof trackedRequest.trackedAt === "number" && now - trackedRequest.trackedAt > REQUEST_TRACK_TTL_MS)
| if (trackedRequest && typeof trackedRequest === "object" && now - trackedRequest.trackedAt > REQUEST_TRACK_TTL_MS) { | |
| if ( | |
| trackedRequest && | |
| typeof trackedRequest === "object" && | |
| typeof trackedRequest.trackedAt === "number" && | |
| now - trackedRequest.trackedAt > REQUEST_TRACK_TTL_MS | |
| ) { |
initialHostByRequestwas only cleaned on terminal webRequest events (onCompleted,onErrorOccurred, block path inonHeadersReceived). When requests are canceled upstream (e.g. by another extension), those events are not guaranteed, leaving stale entries in the tracking map.Problem scope
Tracking model update
{ host, trackedAt }instead of raw host strings.trackedRequest.hostand falling back safely.TTL eviction for orphaned requests
REQUEST_TRACK_TTL_MSand a periodic sweeper (setInterval) that removes entries older than the TTL.onCompleted,onErrorOccurred, redirect/block handling) remain unchanged and continue to remove entries eagerly.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.