fix(geocoder): keep the caller's shape on failure and resolve district results

Two fixes, both verified against a live Photon index.

The error fallback now matches the shape the caller's schema declares.
Query.geocoder is [Geocoder] and Gym.formatted is String, so returning {} made
GraphQL discard the field with "Expected Iterable" or "String cannot represent
value" rather than degrading to an empty answer. Surfacing a non-2xx as a throw
turned a transient Photon 429 or 500 from an empty list into a discarded field,
which is a worse outcome than the silence it replaced. The failure is still
reported: it is logged before the fallback returns.

That object fallback predates this branch and was already wrong for both
callers, but it was only reachable when the URL was missing entirely. Making
upstream failures throw is what turned a rare bug into a routine one.

The type map now includes district. A suburb searched by name comes back as its
own result, with the label in `name` and no `district` field, so the value being
searched for was the one value missing from the answer. Live index, q=West Town
Chicago: {name: "West Town", type: "district", osm_key: "place", osm_value:
"suburb"} now yields suburb "West Town" instead of an empty string.

Both regressions fail their tests without the fix: 34/35 without the district
entry, 32/35 with the object fallback restored.

Author: TurtIeSocks

server/src/services/geocoder.js

@@ -108,7 +108,12 @@ async function geocoder(nominatimUrl, search, reverse, format, provider) {
         : results
   } catch (e) {
     log.warn(TAGS.geocoder, 'Unable to geocode for', search, e)
-    return {}
+    // The fallback has to match the shape the caller's schema expects.
+    // Query.geocoder is [Geocoder] and Gym.formatted is String, so returning {}
+    // made GraphQL discard the field with "Expected Iterable" or "String cannot
+    // represent value" rather than degrading to an empty answer. The failure is
+    // still reported: it is logged immediately above.
+    return reverse ? '' : []
   }
 }
 

server/src/services/photonGeocoder.js

@@ -69,6 +69,11 @@ const TYPE_TO_PROPERTY = {
   state: 'state',
   county: 'county',
   city: 'city',
+  // A search for a suburb by name comes back as type=district with the label in
+  // `name` and no `district` field, so without this the result being asked for
+  // is the one value missing from the answer. Live index, q=West Town Chicago:
+  // {name: "West Town", type: "district", osm_key: "place", osm_value: "suburb"}
+  district: 'district',
   locality: 'locality',
   street: 'street',
 }

server/test/geocoder.test.js

@@ -492,7 +492,7 @@ test('a Photon URL on the Nominatim provider fails without crashing', async () =
     // geocoder() catches and returns {}. What matters is that the process
     // survives to get here at all.
     const result = await geocoder(server.url, 'Denver', false, '{{city}}')
-    assert.deepEqual(result, {})
+    assert.deepEqual(result, [])
   } finally {
     await server.close()
   }
@@ -508,7 +508,7 @@ test('a Nominatim URL on the Photon provider fails without returning nothing sil
       '{{city}}',
       'photon',
     )
-    assert.deepEqual(result, {})
+    assert.deepEqual(result, [])
   } finally {
     await server.close()
   }
@@ -747,3 +747,64 @@ test('a correctly configured Photon webhook still reverse geocodes', async () =>
     await server.close()
   }
 })
+
+// Captured from a live index, q=West Town Chicago. A suburb searched by name
+// comes back as its own result: the label is in `name` and the containing
+// `district` field is absent, so without a district entry in the type map the
+// one value being asked for is the one missing from the answer.
+test('a district searched by name populates suburb', () => {
+  const got = formatPhotonFeature(
+    feature(
+      {
+        city: 'West Chicago Township',
+        country: 'United States',
+        countrycode: 'US',
+        county: 'Cook County',
+        name: 'West Town',
+        osm_key: 'place',
+        osm_value: 'suburb',
+        state: 'Illinois',
+        type: 'district',
+      },
+      [-87.6796, 41.9088],
+    ),
+  )
+  assert.equal(got.suburb, 'West Town')
+  assert.equal(
+    got.formattedAddress,
+    'West Town, West Chicago Township, Cook County, Illinois, United States',
+  )
+})
+
+// Query.geocoder is [Geocoder] and Gym.formatted is String. An object fallback
+// makes GraphQL discard the field entirely rather than degrade to an empty
+// answer, so a transient upstream failure has to keep the caller's shape.
+test('a transient upstream failure keeps the shape the schema expects', async () => {
+  const failing = http.createServer((_, res) => {
+    res.writeHead(500, { 'Content-Type': 'application/json' })
+    res.end(JSON.stringify({ message: 'overloaded' }))
+  })
+  await new Promise((resolve) => {
+    failing.listen(0, '127.0.0.1', resolve)
+  })
+  const url = `http://127.0.0.1:${failing.address().port}`
+  try {
+    const forward = await geocoder(url, 'Denver', false, '{{city}}', 'photon')
+    assert.ok(Array.isArray(forward), `forward returned ${typeof forward}`)
+    assert.deepEqual(forward, [])
+
+    const reverse = await geocoder(
+      url,
+      { lat: 39.7392, lon: -104.9903 },
+      true,
+      '{{city}}',
+      'photon',
+    )
+    assert.equal(typeof reverse, 'string', 'reverse must stay a String')
+    assert.equal(reverse, '')
+  } finally {
+    await new Promise((resolve) => {
+      failing.close(resolve)
+    })
+  }
+})