Implement AbortController for Atlas map request cancellation#485
Merged
Conversation
…bortController for network requests and optimizing data sampling
Contributor
📝 WalkthroughWalkthroughAdds AbortController-driven cancellation and timeout handling across Atlas components and route loaders, propagates signals through tile and soil fetch flows, reduces elevation sampling density (gridSize/chunkSize), removes a stray log, and ensures in-flight requests are aborted and cleaned up to avoid stale processing. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Map UI
participant Atlas as Atlas Component
participant Loader as Route Loader
participant API as External API
User->>Atlas: pan / click → start request
Atlas->>Atlas: abort previous controller (if present)
Atlas->>Loader: fetch tiles/soil (pass AbortSignal)
Loader->>API: fetch data (with combined timeout signal)
API-->>Loader: response or error
alt aborted
Loader->>Atlas: abort detected → stop processing
Atlas-->>User: no update
else success
Loader->>Atlas: processed data
Atlas->>User: render tiles/soil
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/components/blocks/atlas/atlas-sources.tsx (1)
278-290:⚠️ Potential issue | 🟠 MajorCancel the throttled callback in cleanup.
With
trailing: true, a pending throttled call may execute after unmount or dependency change and trigger state updates on a destroyed component.💡 Suggested fix
if (map) { map.on("moveend", throttledLoadData) map.on("zoomend", throttledLoadData) map.once("load", loadData) return () => { map.off("moveend", throttledLoadData) map.off("zoomend", throttledLoadData) + throttledLoadData.cancel() if (abortControllerRef.current) { abortControllerRef.current.abort() } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx` around lines 278 - 290, The throttled callback (throttledLoadData) can fire after unmount because it was created with trailing: true; in the cleanup for the map event listeners (where you remove moveend/zoomend and abortControllerRef.abort()), also call throttledLoadData.cancel() to cancel any pending trailing invocation. Locate the throttle creation (const throttledLoadData = throttle(loadData, 250, { trailing: true })) and add a call to throttledLoadData.cancel() in the returned cleanup function so no pending throttled call runs after the component is torn down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx:
- Around line 337-345: The abort logic that creates/assigns abortControllerRef
and calls abort() must run before the zoom < 13 early return to cancel in-flight
requests; move the block that checks abortControllerRef.current, calls abort(),
then creates a new AbortController and assigns abortControllerRef.current (and
derives signal) so it executes before the zoom threshold guard in the same
function (referencing abortControllerRef and abortController) to ensure stale
tile fetches are cancelled prior to returning.
In
`@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.soil.bodemdata.$soilcode.ts:
- Around line 8-21: The timeout cleanup (clearTimeout(timeoutId)) must run even
if fetch rejects: wrap the fetch call and any use of the composed signal in a
try/finally so clearTimeout(timeoutId) (and any AbortController cleanup)
executes in the finally block; keep creation of timeoutController, timeoutId and
the combined signal (AbortSignal.any([...])) in the same scope, call fetch(...)
inside the try, and move clearTimeout(timeoutId) into the finally so timers
don't remain active after a rejected fetch.
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.soil.tsx:
- Around line 261-282: The current code aborts requests on unmount and when
starting a new request but does not abort an in-flight request when the soil
layer is toggled off; update the onToggleSoil handler to call
abortControllerRef.current?.abort() before clearing state so any inflight fetch
is cancelled and cannot later set selectedSoilFeature. Specifically, in the
onToggleSoil function (the toggle that flips showSoil/clears
selectedSoilFeature), call abortControllerRef.current.abort() (or null-check and
call) then set abortControllerRef.current = null and then clear
selectedSoilFeature and setShowSoil to ensure no stale responses re-populate the
UI. Ensure the existing useEffect cleanup and onMapClick usage of
abortControllerRef remain unchanged.
---
Outside diff comments:
In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx`:
- Around line 278-290: The throttled callback (throttledLoadData) can fire after
unmount because it was created with trailing: true; in the cleanup for the map
event listeners (where you remove moveend/zoomend and
abortControllerRef.abort()), also call throttledLoadData.cancel() to cancel any
pending trailing invocation. Locate the throttle creation (const
throttledLoadData = throttle(loadData, 250, { trailing: true })) and add a call
to throttledLoadData.cancel() in the returned cleanup function so no pending
throttled call runs after the component is torn down.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/cool-adults-report.mdfdm-app/app/components/blocks/atlas/atlas-sources.tsxfdm-app/app/integrations/ahn-cache.server.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.soil.bodemdata.$soilcode.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.soil.tsx
💤 Files with no reviewable changes (1)
- fdm-app/app/integrations/ahn-cache.server.ts
Ensures stale network requests are cancelled before zoom guards, guarantees timeout cleanup in the soil loader, and cancels pendingthrottled calls on unmount to prevent race conditions and memory leaks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (1)
337-342:⚠️ Potential issue | 🟠 MajorReset loading/status on zoom-out abort path to avoid stuck UI state.
When a running request is aborted by a zoom-out call, the aborted request skips
setIsUpdating(false)infinally, and the zoom-out branch returns without resetting state. This can leave loading indicators stuck.Suggested fix
if (zoom < 13) { if (activeTilesLengthRef.current > 0) { setActiveTiles([]) } + setIsUpdating(false) + setNetworkStatus("idle") return }Also applies to: 510-511
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx around lines 337 - 342, The zoom-out branch that returns early when zoom < 13 currently clears active tiles via setActiveTiles([]) but doesn’t reset the request state, causing loading indicators to stick; update that branch in farm.$b_id_farm.$calendar.atlas.elevation (the block that checks `if (zoom < 13)`) to also call the relevant state-resetters (e.g., `setIsUpdating(false)` and any other loading flags you use alongside `setIsUpdating`) so aborted requests don’t leave the UI in a loading state, and apply the same change to the analogous early-return block around the code at the other occurrence (the block referenced by lines ~510-511).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx`:
- Around line 216-223: Move the request-cancellation block so it runs before any
early-return/branch checks: call abortControllerRef.current?.abort(), create a
new AbortController and assign it to abortControllerRef.current (and extract
signal) at the start of the function/run that may early-exit, not only inside
the high-zoom/bounds branch. This ensures any previous in-flight request is
aborted even if the current run returns early; update uses of abortController/
signal accordingly (look for abortControllerRef, abortController, and signal in
the function).
---
Outside diff comments:
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx:
- Around line 337-342: The zoom-out branch that returns early when zoom < 13
currently clears active tiles via setActiveTiles([]) but doesn’t reset the
request state, causing loading indicators to stick; update that branch in
farm.$b_id_farm.$calendar.atlas.elevation (the block that checks `if (zoom <
13)`) to also call the relevant state-resetters (e.g., `setIsUpdating(false)`
and any other loading flags you use alongside `setIsUpdating`) so aborted
requests don’t leave the UI in a loading state, and apply the same change to the
analogous early-return block around the code at the other occurrence (the block
referenced by lines ~510-511).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/components/blocks/atlas/atlas-sources.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.soil.bodemdata.$soilcode.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.soil.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (1)
332-339: Create the newAbortControlleronly when zoom is eligible for tile requests.You already correctly abort the previous request before the zoom guard. A small cleanup: move creation/assignment of the new controller to after the zoom >= 13 check so zoomed-out updates don’t allocate unused controllers.
Refactor sketch
// Cancel previous request if (abortControllerRef.current) { abortControllerRef.current.abort() } - const abortController = new AbortController() - abortControllerRef.current = abortController - const signal = abortController.signal // If zoomed out, clear active tiles to save resources (WMS will take over) if (zoom < 13) { if (activeTilesLengthRef.current > 0) { setActiveTiles([]) } setIsUpdating(false) setNetworkStatus("idle") return } + + const abortController = new AbortController() + abortControllerRef.current = abortController + const signal = abortController.signalAlso applies to: 341-348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx around lines 332 - 339, Move creation/assignment of the new AbortController so it only happens when the zoom check allows tile requests: keep the existing abort of abortControllerRef.current before the zoom guard, then after verifying zoom >= 13 (the guard that prevents requests when zoomed out) create const abortController = new AbortController(), assign abortControllerRef.current = abortController and extract signal from abortController.signal; apply the same change for the second occurrence around lines with abortControllerRef/AbortController in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx`:
- Line 237: deserialize(availableFieldsUrl, bbox) performs an uncancelable
internal fetch; wrap the HTTP request with fetch(availableFieldsUrl, { signal })
and pass response.body (a ReadableStream) into deserialize to allow
AbortController cancellation. Specifically, replace the direct call to
deserialize(availableFieldsUrl, bbox) with a pattern that calls
fetch(availableFieldsUrl, { signal }), checks response.ok, obtains
response.body, and then calls deserialize(response.body, bbox); keep existing
signal.aborted checks and ensure any thrown errors from fetch or deserialize are
handled so aborted requests don't proceed.
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx:
- Around line 323-327: The early-return branch in updateVisibleTiles that checks
mapRef.current and indexData should not reset network status; remove or stop
calling setNetworkStatus("idle") inside that if-block so you only call
setIsUpdating(false) and return when prerequisites are missing (refer to the
mapRef, indexData, setIsUpdating, setNetworkStatus, and updateVisibleTiles
symbols) to avoid overwriting real loading/error states from the index fetch.
---
Nitpick comments:
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.atlas.elevation.tsx:
- Around line 332-339: Move creation/assignment of the new AbortController so it
only happens when the zoom check allows tile requests: keep the existing abort
of abortControllerRef.current before the zoom guard, then after verifying zoom
>= 13 (the guard that prevents requests when zoomed out) create const
abortController = new AbortController(), assign abortControllerRef.current =
abortController and extract signal from abortController.signal; apply the same
change for the second occurrence around lines with
abortControllerRef/AbortController in the file.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/atlas/atlas-sources.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/atlas/atlas-sources.tsx (1)
238-238:⚠️ Potential issue | 🟠 MajorURL-based FlatGeobuf deserialization still bypasses true transport cancellation.
At Line 238,
deserialize(availableFieldsUrl, bbox)keeps fetch ownership inside FlatGeobuf. Yoursignal.abortedchecks prevent stale state writes, but may still allow the underlying network transfer to continue.Suggested fix
- const iter = deserialize(availableFieldsUrl, bbox) + const response = await fetch(availableFieldsUrl, { + signal, + }) + if (!response.ok || !response.body) { + throw new Error( + `Failed to fetch fields: ${response.status}`, + ) + } + const iter = deserialize(response.body, bbox)For the exact flatgeobuf version used in this repo, does `deserialize(url, rect)` support AbortSignal-backed HTTP cancellation, or is `fetch(url, { signal })` + `deserialize(response.body, rect)` the recommended abortable pattern?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx` at line 238, The current call const iter = deserialize(availableFieldsUrl, bbox) hands a URL to FlatGeobuf which may not honor our AbortSignal; instead perform a fetch(availableFieldsUrl, { signal }) to get a Response and pass response.body (the ReadableStream) into deserialize(response.body, bbox) so network reads are cancelable by the existing signal; update the usage site that references availableFieldsUrl, bbox and the local AbortSignal (e.g., signal or abortController.signal) to use this fetch-then-deserialize pattern and close/cleanup the response if aborted or on error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@fdm-app/app/components/blocks/atlas/atlas-sources.tsx`:
- Line 238: The current call const iter = deserialize(availableFieldsUrl, bbox)
hands a URL to FlatGeobuf which may not honor our AbortSignal; instead perform a
fetch(availableFieldsUrl, { signal }) to get a Response and pass response.body
(the ReadableStream) into deserialize(response.body, bbox) so network reads are
cancelable by the existing signal; update the usage site that references
availableFieldsUrl, bbox and the local AbortSignal (e.g., signal or
abortController.signal) to use this fetch-then-deserialize pattern and
close/cleanup the response if aborted or on error.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/atlas/atlas-sources.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
This was referenced Mar 13, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Performance
Chores