From 261d11f29d424cfba273ad6586cb1a0996c4fd31 Mon Sep 17 00:00:00 2001 From: jmcqueen Date: Thu, 9 Jul 2026 12:35:00 -0400 Subject: [PATCH] Harden /finalizeStore against pending-order and DECT-404 noise A finalize run against a store whose phone number still had a pending carrier order surfaced three unrelated-looking 400s (add-number, caller ID = LOCATION_NUMBER, auto attendant "number already used") that all traced back to the number not being attached, plus a spurious "DECT network pre-check failed" warn when the location had never had a DECT network before. Fixes: - addPhoneNumbersToLocation now translates the raw NUMBER_HAS_PENDING_ORDERS payload into an actionable operator message ("wait for the carrier order, then re-run /finalizeStore"). Original error preserved as .cause. Extracted as pure translateAddNumberError so it can be unit-tested. - finalizeStore marks the phone-number-add step { critical: true } so finalize aborts immediately on that failure instead of cascading three downstream errors that bury the root cause. - findDectNetworkInLocation treats HTTP 404 as "no networks yet" (Webex returns 404 for that state, not an empty list), so the finalize pre-check stops warning on the normal first-time path. The remaining warn now includes status code for anything that does reach it. Tests: 4 new cases in test/locations.test.js locking down the translator behavior against the exact Webex payload shape. Co-authored-by: Cursor --- src/flows/finalizeStore.js | 20 +++++++++++-- src/webex/dect.js | 13 ++++++++- src/webex/locations.js | 42 +++++++++++++++++++++++---- test/locations.test.js | 59 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 test/locations.test.js diff --git a/src/flows/finalizeStore.js b/src/flows/finalizeStore.js index 0bf5c08..d0408ed 100644 --- a/src/flows/finalizeStore.js +++ b/src/flows/finalizeStore.js @@ -42,8 +42,16 @@ export async function finalizeStoreLocation(bot, locationInfo, userInfo) { } const location = matches[0]; - await runStep(bot, 'Added phone numbers to location', () => - addPhoneNumbersToLocation(location, locationInfo.phoneNumber), + // Critical: every downstream step (calling identity, per-user + // caller ID = LOCATION_NUMBER, auto attendant on this DID) needs + // the number attached to the location. If this fails there is no + // point cascading the rest and burying the root cause in noise — + // bail early so the actionable Webex message reaches the operator. + await runStep( + bot, + 'Added phone numbers to location', + () => addPhoneNumbersToLocation(location, locationInfo.phoneNumber), + { critical: true }, ); await runStep(bot, 'Updated location Webex Calling Details', () => updateLocationCallingIdentity(location, locationInfo.phoneNumber), @@ -105,7 +113,13 @@ async function ensureDectNetwork(bot, location, storeNumber) { return existing; } } catch (error) { - logger.warn(`DECT network pre-check failed for ${netName}: ${error.message}`); + // 404s are handled inside findDectNetworkInLocation (return + // null), so anything reaching us here is a real problem worth + // flagging in the log. We still fall through to try creating, + // since a transient read failure shouldn't block finalize. + logger.warn( + `DECT network pre-check failed for ${netName} (status=${error?.status ?? '?'}): ${error?.message}`, + ); } // The default access code (the "DECT pin") is derived from the store // number via generateDectAccessCode — 4-digit stores use their own diff --git a/src/webex/dect.js b/src/webex/dect.js index 75961a6..fd22c2f 100644 --- a/src/webex/dect.js +++ b/src/webex/dect.js @@ -114,7 +114,18 @@ export async function findDectNetworkForStore(storeNumber) { */ export async function findDectNetworkInLocation(locationId, storeNumber) { if (!locationId) return null; - const data = await webexJson('GET', `/telephony/config/locations/${locationId}/dectNetworks`); + let data; + try { + data = await webexJson('GET', `/telephony/config/locations/${locationId}/dectNetworks`); + } catch (err) { + // Webex returns 404 ("No static resource ... /dectNetworks") + // when a location has never had a DECT network — as opposed + // to returning an empty list. Treat that as "no networks + // yet" so the caller can proceed to create one without a + // spurious warning on the finalize output. + if (err?.status === 404) return null; + throw err; + } const items = data?.dectNetworks ?? data?.items ?? []; const target = dectNetworkName(storeNumber).toLowerCase(); return items.find((n) => (n.name ?? '').trim().toLowerCase() === target) ?? null; diff --git a/src/webex/locations.js b/src/webex/locations.js index 7226fad..33d1165 100644 --- a/src/webex/locations.js +++ b/src/webex/locations.js @@ -56,13 +56,45 @@ export async function enableLocationForCalling(location) { return webexJson('POST', '/telephony/config/locations', body); } +/** + * Rewrite the raw Webex error from POST /locations/.../numbers into an + * operator-actionable one for known transient failure modes. Returns + * the original error unchanged for anything we don't have a friendlier + * message for. Exported so it can be unit-tested without touching the + * network. + * + * Known modes handled: + * - NUMBER_HAS_PENDING_ORDERS (Webex code ERR.V.TRM.TMN60027): the + * carrier's port / provisioning order is still in flight. The fix + * is "wait a few minutes and re-run finalize", not anything the + * operator can debug from the stack trace. Preserves .status and + * wraps the original as .cause for programmatic introspection. + */ +export function translateAddNumberError(err, phoneNumber) { + if (typeof err?.body === 'string' && err.body.includes('NUMBER_HAS_PENDING_ORDERS')) { + const clearer = new Error( + `Phone number ${phoneNumber} still has a pending order at the carrier — ` + + 'it is not yet attachable to a Webex location. Wait a few minutes for ' + + 'the order to complete in Control Hub, then re-run /finalizeStore.', + ); + clearer.cause = err; + clearer.status = err.status; + return clearer; + } + return err; +} + export async function addPhoneNumbersToLocation(location, phoneNumber) { const body = { phoneNumbers: [phoneNumber], state: 'ACTIVE' }; - return webexJson( - 'POST', - `/telephony/config/locations/${encodeURIComponent(location.id)}/numbers`, - body, - ); + try { + return await webexJson( + 'POST', + `/telephony/config/locations/${encodeURIComponent(location.id)}/numbers`, + body, + ); + } catch (err) { + throw translateAddNumberError(err, phoneNumber); + } } /** diff --git a/test/locations.test.js b/test/locations.test.js new file mode 100644 index 0000000..20df685 --- /dev/null +++ b/test/locations.test.js @@ -0,0 +1,59 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { translateAddNumberError } from '../src/webex/locations.js'; + +// The exact JSON Webex returns when a freshly-purchased number is +// still being provisioned by the carrier. Kept verbatim from a real +// error so this test breaks loudly if Webex ever changes the payload +// shape (and we can update our detection to match). +const PENDING_ORDERS_BODY = + '[{"errors":[{"errorType":"VERIFICATION","errorCode":"ERR.V.TRM.TMN60027","errorMessage":"Number has pending orders.","errorTitle":"NUMBER_HAS_PENDING_ORDERS"}]}]'; + +function fakeHttpError(status, body) { + const err = new Error(`HTTP ${status} for POST /whatever\n${body}`); + err.status = status; + err.body = body; + return err; +} + +describe('translateAddNumberError', () => { + // Regression: without the translation, the raw Webex error was + // propagated through runStep, cascaded into a Caller-ID failure + // (LOCATION_NUMBER unavailable) and an auto-attendant failure + // (number "used in another location"), and the actionable root + // cause was buried under three stack traces. + it('rewrites NUMBER_HAS_PENDING_ORDERS into an actionable operator message', () => { + const raw = fakeHttpError(400, PENDING_ORDERS_BODY); + const translated = translateAddNumberError(raw, '+14045551212'); + assert.notEqual(translated, raw, 'should be a new Error instance'); + assert.match(translated.message, /pending order/i); + assert.match(translated.message, /re-run \/finalizeStore/); + assert.match(translated.message, /\+14045551212/); + assert.equal(translated.status, 400); + assert.equal(translated.cause, raw, 'raw error should be preserved as .cause'); + }); + + it('returns unrelated errors unchanged (same instance, no message rewrite)', () => { + const raw = fakeHttpError(400, '{"message":"Nope","errorCode":9999}'); + const result = translateAddNumberError(raw, '+14045551212'); + assert.equal(result, raw, 'must be same instance so callers can rethrow'); + assert.doesNotMatch(result.message, /pending order/i); + }); + + it('tolerates errors with no body property (e.g. transport failures)', () => { + const raw = new Error('ENOTFOUND webexapis.com'); + const result = translateAddNumberError(raw, '+14045551212'); + assert.equal(result, raw); + }); + + it('does not misfire on other 400 payloads that happen to be arrays', () => { + // Same array-of-errors shape but a different error code. + const raw = fakeHttpError( + 400, + '[{"errors":[{"errorCode":"ERR.OTHER","errorMessage":"unrelated"}]}]', + ); + const result = translateAddNumberError(raw, '+14045551212'); + assert.equal(result, raw); + }); +});