fix(geocoder): report a Photon URL left on the Nominatim provider

Reproduced against a live deployment. A webhook that inherits its URL from
Poracle and sets no geocoderProvider returned a single blank result, with
nothing in the log to explain it.

node-geocoder never gets far enough to receive a FeatureCollection, so the
previous check could not fire. It calls /search, which Photon does not serve,
and _forceParams puts format and addressdetails on every request, which Photon
rejects outright. A live instance answers:

  GET /search?q=..&format=json&addressdetails=1
    404 {"title":"Endpoint GET /search not found","status":404,...}
  GET /reverse?lat=..&lon=..&format=json&addressdetails=1
    400 {"message":"Unknown query parameter 'format'.  Allowed parameters
         are: [include, debug, dedupe, ...]"}

node-geocoder ignores the status and parses the body regardless, so both reach
_formatResult as an object with no address and format into an empty string. Both
shapes are now recognised, and the error names the setting that fixes it.

The regression server enforces Photon's actual contract rather than accepting
anything: it serves /api and /reverse only, answers 404 elsewhere with Javalin's
body, and rejects any parameter outside the allow list.

Separately, locality reached no consumer. node-geocoder emits `neighbourhood`,
the GraphQL schema exposes `neighborhood` and formatter() templates on
`neighborhoods`, so the value was invisible to raw clients and to configured
address formats alike. Both providers now carry the alias and formatter accepts
either spelling, which also makes the field work for Nominatim deployments,
where it had been dead for the same reason.

Author: TurtIeSocks

server/src/services/geocoder.js

@@ -12,7 +12,7 @@ const { photonGeocoder } = require('./photonGeocoder')
 function formatter(addressFormat, result) {
   return addressFormat
     .replace(
-      /{{(streetNumber|streetName|city|state|country|zipcode|latitude|longitude|countryCode|neighborhoods|suburb|town|village)}}/g,
+      /{{(streetNumber|streetName|city|state|country|zipcode|latitude|longitude|countryCode|neighborhoods|neighborhood|neighbourhood|suburb|town|village)}}/g,
       (a, b) => result[b] || '',
     )
     .trim()
@@ -38,9 +38,33 @@ function formatter(addressFormat, result) {
  */
 function assertNominatimResponse(results, url) {
   const raw = results?.raw
-  if (raw && !Array.isArray(raw) && raw.type === 'FeatureCollection') {
+  if (!raw || Array.isArray(raw) || typeof raw !== 'object') return
+
+  // Photon rarely gets far enough to answer with GeoJSON here, because
+  // node-geocoder asks for routes and parameters it does not serve. Verified
+  // against a live instance:
+  //
+  //   GET /search?q=..&format=json&addressdetails=1
+  //     404 {"title":"Endpoint GET /search not found","status":404,...}
+  //     Photon serves /api, not /search.
+  //
+  //   GET /reverse?lat=..&lon=..&format=json&addressdetails=1
+  //     400 {"message":"Unknown query parameter 'format'. Allowed parameters
+  //          are: [include, debug, dedupe, ...]"}
+  //     Photon rejects anything outside its allow list, and format and
+  //     addressdetails are forced onto every request by node-geocoder.
+  //
+  // node-geocoder ignores the status and parses the body regardless, so both
+  // arrive here as an object with no address and format into a blank result.
+  const isPhoton =
+    raw.type === 'FeatureCollection' ||
+    (typeof raw.title === 'string' && typeof raw.status === 'number') ||
+    (typeof raw.message === 'string' &&
+      raw.message.includes('Unknown query parameter'))
+
+  if (isPhoton) {
     throw new Error(
-      `${url} answered with GeoJSON, which is Photon's format rather than Nominatim's. Set "geocoderProvider": "photon" on this webhook, or point the URL at a Nominatim instance.`,
+      `${url} answered as a Photon instance rather than a Nominatim one. Set "geocoderProvider": "photon" on this webhook, or point the URL at a Nominatim instance.`,
     )
   }
 }
@@ -57,12 +81,20 @@ async function nominatimGeocoder(url, search, isReverse) {
     osmServer: url,
     timeout: 5000,
   })
-  stockGeocoder._geocoder._formatResult = ((original) => (result) => ({
-    ...original(result),
-    suburb: result.address?.suburb || '',
-    town: result.address?.town || '',
-    village: result.address?.village || '',
-  }))(stockGeocoder._geocoder._formatResult)
+  stockGeocoder._geocoder._formatResult = ((original) => (result) => {
+    const formatted = original(result)
+    return {
+      ...formatted,
+      suburb: result.address?.suburb || '',
+      town: result.address?.town || '',
+      village: result.address?.village || '',
+      // node-geocoder emits the British spelling. The GraphQL schema exposes
+      // `neighborhood` and formatter() templates on `neighborhoods`, so the
+      // value reached neither consumer. Carrying the alias is what makes it
+      // visible without changing what node-geocoder itself produces.
+      neighborhood: formatted.neighbourhood || '',
+    }
+  })(stockGeocoder._geocoder._formatResult)
   // Awaited rather than returned so the shape check runs here. A throw inside
   // _formatResult would not reach geocoder()'s catch at all: node-geocoder
   // resolves through bluebird's asCallback, so it surfaces as an uncaught
@@ -117,4 +149,4 @@ async function geocoder(nominatimUrl, search, reverse, format, provider) {
   }
 }
 
-module.exports = { geocoder, formatter }
+module.exports = { geocoder, formatter, nominatimGeocoder }

server/src/services/photonGeocoder.js

@@ -247,6 +247,11 @@ function formatPhotonFeature(feature) {
     // same building, Photon's locality and district carry the values Nominatim
     // returns as quarter and suburb.
     neighbourhood: properties.locality || '',
+    // The same value under the spelling the GraphQL schema and formatter use.
+    // Geocoder.neighborhood is American and formatter() templates on
+    // neighborhoods, while node-geocoder emits neighbourhood, so without the
+    // alias the mapped value reaches no consumer at all.
+    neighborhood: properties.locality || '',
     suburb: properties.district || '',
     town: town || '',
     village: village || '',

server/test/geocoder.test.js

@@ -6,7 +6,11 @@ const NodeGeocoder = require('node-geocoder')
 const http = require('node:http')
 
 const { PoracleAPI } = require('../src/services/Poracle')
-const { geocoder } = require('../src/services/geocoder')
+const {
+  formatter,
+  geocoder,
+  nominatimGeocoder,
+} = require('../src/services/geocoder')
 const {
   formatPhotonFeature,
   joinComponents,
@@ -59,6 +63,7 @@ test('maps a Photon city onto the geocoder entry shape', () => {
     streetNumber: undefined,
     countryCode: 'US',
     neighbourhood: '',
+    neighborhood: '',
     suburb: '',
     town: '',
     village: '',
@@ -374,12 +379,18 @@ test('the Photon path emits the same keys as the Nominatim path', () => {
     osmServer: 'http://127.0.0.1:0',
     timeout: 5000,
   })
-  stockGeocoder._geocoder._formatResult = ((original) => (result) => ({
-    ...original(result),
-    suburb: result.address.suburb || '',
-    town: result.address.town || '',
-    village: result.address.village || '',
-  }))(stockGeocoder._geocoder._formatResult.bind(stockGeocoder._geocoder))
+  // Mirrors the patch in geocoder.js, alias included. The duplication is the
+  // point: if the two drift apart, this test stops proving parity.
+  stockGeocoder._geocoder._formatResult = ((original) => (result) => {
+    const formatted = original(result)
+    return {
+      ...formatted,
+      suburb: result.address?.suburb || '',
+      town: result.address?.town || '',
+      village: result.address?.village || '',
+      neighborhood: formatted.neighbourhood || '',
+    }
+  })(stockGeocoder._geocoder._formatResult.bind(stockGeocoder._geocoder))
 
   // The same place as STREET_ADDRESS, in Nominatim's response shape.
   const fromNominatim = stockGeocoder._geocoder._formatResult({
@@ -808,3 +819,142 @@ test('a transient upstream failure keeps the shape the schema expects', async ()
     })
   }
 })
+
+// Behaves like a real Photon instance rather than accepting anything: it serves
+// /api and /reverse only, and rejects any parameter outside its allow list.
+// node-geocoder forces format and addressdetails onto every request and calls
+// /search, so a Photon URL left on the Nominatim provider never reaches a
+// FeatureCollection at all. Captured from a live instance.
+const servePhoton = async () => {
+  const ALLOWED = new Set([
+    'include',
+    'debug',
+    'dedupe',
+    'query_string_filter',
+    'lon',
+    'layer',
+    'limit',
+    'osm_tag',
+    'distance_sort',
+    'geometry',
+    'exclude',
+    'lang',
+    'radius',
+    'lat',
+    'q',
+  ])
+  const server = http.createServer((req, res) => {
+    const url = new URL(req.url, 'http://127.0.0.1')
+    const json = (code, body) => {
+      res.writeHead(code, { 'Content-Type': 'application/json' })
+      res.end(JSON.stringify(body))
+    }
+    if (url.pathname !== '/api' && url.pathname !== '/reverse') {
+      json(404, {
+        title: `Endpoint ${req.method} ${url.pathname} not found`,
+        status: 404,
+        type: 'https://javalin.io/documentation#endpointnotfound',
+        details: {},
+      })
+      return
+    }
+    const bad = [...url.searchParams.keys()].find((k) => !ALLOWED.has(k))
+    if (bad) {
+      json(400, {
+        message: `Unknown query parameter '${bad}'.  Allowed parameters are: [${[...ALLOWED].join(', ')}]`,
+      })
+      return
+    }
+    json(200, { type: 'FeatureCollection', features: [] })
+  })
+  await new Promise((resolve) => {
+    server.listen(0, '127.0.0.1', resolve)
+  })
+  return {
+    url: `http://127.0.0.1:${server.address().port}`,
+    close: () =>
+      new Promise((resolve) => {
+        server.close(resolve)
+      }),
+  }
+}
+
+// The exact production misconfiguration: a Photon URL inherited from Poracle
+// with no geocoderProvider set. It used to format into a single blank result.
+test('a Photon URL on the Nominatim provider is reported on the forward path', async () => {
+  const server = await servePhoton()
+  try {
+    await assert.rejects(
+      () => nominatimGeocoder(server.url, 'Denver', false),
+      /answered as a Photon instance/,
+    )
+  } finally {
+    await server.close()
+  }
+})
+
+test('a Photon URL on the Nominatim provider is reported on the reverse path', async () => {
+  const server = await servePhoton()
+  try {
+    await assert.rejects(
+      () =>
+        nominatimGeocoder(server.url, { lat: 39.7392, lon: -104.9903 }, true),
+      /answered as a Photon instance/,
+    )
+  } finally {
+    await server.close()
+  }
+})
+
+// The value has to reach the two consumers that actually exist: the GraphQL
+// field is `neighborhood` and formatter() templates on `neighborhoods`, while
+// node-geocoder produces `neighbourhood`.
+test('locality reaches the public neighborhood contract', () => {
+  const got = formatPhotonFeature(
+    feature(WICKER_PARK_REVERSE, [-87.6796, 41.9088]),
+  )
+  assert.equal(got.neighbourhood, 'Wicker Park')
+  assert.equal(got.neighborhood, 'Wicker Park')
+  assert.equal(formatter('{{neighborhood}}', got), 'Wicker Park')
+  assert.equal(formatter('{{neighbourhood}}', got), 'Wicker Park')
+})
+
+// The alias has to exist on the Nominatim path too. node-geocoder emits
+// neighbourhood there, and the GraphQL field and formatter token both use the
+// American spellings, so a Nominatim deployment had the same invisible value.
+test('the Nominatim path also carries the neighborhood alias', async () => {
+  const server = http.createServer((_, res) => {
+    res.writeHead(200, { 'Content-Type': 'application/json' })
+    res.end(
+      JSON.stringify([
+        {
+          lat: '41.9088',
+          lon: '-87.6796',
+          display_name: 'Wicker Park, Chicago',
+          address: {
+            neighbourhood: 'Wicker Park',
+            city: 'Chicago',
+            country_code: 'us',
+          },
+        },
+      ]),
+    )
+  })
+  await new Promise((resolve) => {
+    server.listen(0, '127.0.0.1', resolve)
+  })
+  try {
+    const [entry] = await nominatimGeocoder(
+      `http://127.0.0.1:${server.address().port}`,
+      'Wicker Park',
+      false,
+    )
+    assert.equal(entry.neighbourhood, 'Wicker Park')
+    assert.equal(entry.neighborhood, 'Wicker Park')
+    assert.equal(formatter('{{neighborhood}}', entry), 'Wicker Park')
+  } finally {
+    await new Promise((resolve) => {
+      server.close(resolve)
+    })
+  }
+})