Harden /finalizeStore against pending-order and DECT-404 noise
Some checks are pending
CI / verify (push) Waiting to run
Some checks are pending
CI / verify (push) Waiting to run
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 <cursoragent@cursor.com>
This commit is contained in:
parent
203850fd69
commit
261d11f29d
4 changed files with 125 additions and 9 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
try {
|
||||
return await webexJson(
|
||||
'POST',
|
||||
`/telephony/config/locations/${encodeURIComponent(location.id)}/numbers`,
|
||||
body,
|
||||
);
|
||||
} catch (err) {
|
||||
throw translateAddNumberError(err, phoneNumber);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
59
test/locations.test.js
Normal file
59
test/locations.test.js
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue