fix: cover geocode response bodies with deadline (#160)

Author: atomantic

server/src/routes/map.routes.ts

@@ -94,7 +94,7 @@ mapRouter.get('/geocode/stream', async (req: Request, res: Response) => {
     return 'error';
   });
 
-  if (streamResult === 'ok') {
+  if (streamResult === 'ok' && !cancelled) {
     sendEvent({ type: 'complete', current: placesToGeocode.length, total: placesToGeocode.length });
   }
 

server/src/services/geocode.service.ts

@@ -97,8 +97,9 @@ function ensurePending(placeText: string): void {
 }
 
 type FetchResult = { status: 'found'; result: NominatimResult } | { status: 'not_found' } | { status: 'error' } | { status: 'cancelled' };
+type NominatimResponse = { status: number; ok: boolean; data: NominatimResult[] };
 
-async function fetchWithDeadline(url: string, query: string, callerSignal?: AbortSignal): Promise<Response | null | 'cancelled'> {
+async function fetchWithDeadline(url: string, query: string, callerSignal?: AbortSignal): Promise<NominatimResponse | null | 'cancelled'> {
   if (callerSignal?.aborted) return 'cancelled';
 
   const controller = new AbortController();
@@ -111,7 +112,13 @@ async function fetchWithDeadline(url: string, query: string, callerSignal?: Abor
   }, REQUEST_TIMEOUT_MS);
 
   try {
-    return await fetch(url, { headers: { 'User-Agent': USER_AGENT }, signal: controller.signal });
+    const response = await fetch(url, { headers: { 'User-Agent': USER_AGENT }, signal: controller.signal });
+    if (response.status === 429) {
+      void response.body?.cancel().catch(() => {});
+      return { status: response.status, ok: response.ok, data: [] };
+    }
+    const data = response.ok ? await response.json() as NominatimResult[] : [];
+    return { status: response.status, ok: response.ok, data };
   } catch (error) {
     if (callerSignal?.aborted) {
       logger.warn('geocode', `Geocode request status=cancelled place=${JSON.stringify(query)} reason=caller_cancelled`);
@@ -147,13 +154,11 @@ export function fetchNominatim(query: string, signal?: AbortSignal): Promise<Fet
         const retry = await fetchWithDeadline(url, query, signal);
         if (retry === 'cancelled') return { status: 'cancelled' };
         if (!retry?.ok) return { status: 'error' };
-        const retryData: NominatimResult[] = await retry.json();
-        return retryData[0] ? { status: 'found', result: retryData[0] } : { status: 'not_found' };
+        return retry.data[0] ? { status: 'found', result: retry.data[0] } : { status: 'not_found' };
       }
 
       if (!response.ok) return { status: 'error' };
-      const data: NominatimResult[] = await response.json();
-      return data[0] ? { status: 'found', result: data[0] } : { status: 'not_found' };
+      return response.data[0] ? { status: 'found', result: response.data[0] } : { status: 'not_found' };
     } catch (error) {
       if (signal?.aborted) return { status: 'cancelled' };
       logger.warn('geocode', `Geocode response status=error place=${JSON.stringify(query)} reason=response_error`);

tests/unit/routes/mapGeocodeStream.spec.ts

@@ -24,7 +24,6 @@ describe('map geocode stream cancellation', () => {
     batchGeocode.mockImplementation(async function* (_places: string[], signal?: AbortSignal) {
       receivedSignal = signal;
       await new Promise<void>(resolve => signal?.addEventListener('abort', () => resolve(), { once: true }));
-      yield { type: 'progress', current: 1, total: 2, place: 'First place', status: 'resolved' };
     });
 
     const layer = (mapRouter as unknown as { stack: Array<{ route?: { path: string; stack: Array<{ handle: (req: unknown, res: unknown) => Promise<void> }> } }> }).stack

tests/unit/services/geocode.service.spec.ts

@@ -49,6 +49,21 @@ describe('geocodeService Nominatim lifecycle', () => {
     expect(fetchMock).toHaveBeenCalledTimes(2);
   });
 
+  it('keeps the deadline active while reading a stalled response body', async () => {
+    const fetchMock = vi.fn((_url: string, init: RequestInit) => ({
+      status: 200,
+      ok: true,
+      json: () => new Promise((_resolve, reject) => {
+        init.signal?.addEventListener('abort', () => reject(new DOMException('Timed out', 'AbortError')));
+      }),
+    }));
+    vi.stubGlobal('fetch', fetchMock);
+
+    const request = fetchNominatim('Slow body place');
+    await vi.advanceTimersByTimeAsync(1_100 + 15_000);
+    await expect(request).resolves.toEqual({ status: 'error' });
+  });
+
   it('does not make another upstream request after a batch is cancelled', async () => {
     const fetchMock = vi.fn((_url: string, init: RequestInit) => new Promise((_resolve, reject) => {
       init.signal?.addEventListener('abort', () => reject(new DOMException('Cancelled', 'AbortError')));