improve(serverless-orchestration): let bots opt out of hub paging - #4965
improve(serverless-orchestration): let bots opt out of hub paging#4965nicholaspai wants to merge 2 commits into
Conversation
The hub logs every spoke failure at `error`, which the PagerDuty transport picks up and pages on. A single testnet bot failing in a shared config file therefore pages the on-call for the whole across-bots service. A bot config can now set `pageOnError: false`. When every failing bot in a run carries the flag the hub logs at `warn` instead: Slack and GCP logging still receive it, PagerDuty does not. Any run with an unflagged failing bot, or a failure the hub cannot attribute to a bot, pages exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| // The PagerDuty transport only accepts `error`; `warn` still reaches Slack and GCP logging. Failures we | ||
| // can't attribute to a bot page anyway. | ||
| const failedBots = Object.keys(errorOutput?.errorOutputs ?? {}); | ||
| const pages = !failedBots.length || failedBots.some((bot) => configObject?.[bot]?.pageOnError !== false); |
There was a problem hiding this comment.
This covers spoke-attributable failures, but the errorOutput instanceof Error branch just above still calls logger.error(..., notificationPath: "infrastructure-error") unconditionally, so pageOnError: false has no effect there.
That branch is reachable from the same root cause this PR is trying to silence. The pre-dispatch loop in this same try calls _getChainId() and _getLatestBlockNumber() against each bot's own NODE_RETRY_CONFIG/CUSTOM_NODE_URL (and _getBlockNumberOnChainIdMultiChain for NODE_URLS_<chainId>). viem transport errors extend Error, so if that QuickNode endpoint degrades for eth_chainId/eth_blockNumber rather than only eth_call, the run aborts into the fatal branch and pages anyway — and no bot in the file gets dispatched at all.
Worth either consulting pageOnError in the Error branch when the failure is attributable to a single bot, or catching the per-bot block-number fetch so a dead endpoint surfaces as that bot's own spoke error instead of a hub-fatal one.
md0x
left a comment
There was a problem hiding this comment.
LGTM
Just added a medium finding found with Claude.
…lure pageOnError read as though it controlled the bot's own PagerDuty transport, which is a separate flag inside environmentVariables. The name now says which pager it flips, and "failure" matches the condition the hub actually reacts to (spoke timeout, rejection, non-zero exit, empty stdout). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: nicholaspai <npai.nyc@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-out mechanism so individual bot configs can suppress hub-level PagerDuty paging on spoke failures by downgrading the hub log level from error to warn when all failing bots in a run set hubPageOnFailure: false. This addresses noisy paging for non-critical bots while preserving Slack/GCP logging and preserving paging whenever any failing bot is not opted out.
Changes:
- Introduces
hubPageOnFailureevaluation inServerlessHubto selectlogger.errorvslogger.warnfor spoke-failure aggregation logs. - Hoists the fetched
configObjectso the catch block can determine per-bot paging behavior. - Adds/updates
ServerlessHubtests to validate opt-out behavior and the “mixed criticality” guardrail.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/serverless-orchestration/src/ServerlessHub.js | Compute whether hub should page based on failing bots’ hubPageOnFailure and log at warn when paging is suppressed. |
| packages/serverless-orchestration/test/ServerlessHub.js | Adds tests asserting warn-level logging when all failing bots opt out, and error-level logging when any failing bot does not. |
Suppressed comments (1)
packages/serverless-orchestration/test/ServerlessHub.js:349
- This test also starts a second hub server and leaves it running. Please close the returned server instance (ideally in a
finally) to avoid leaking listeners/open handles between tests.
const testHubPort = 8086; // create a separate port to run this specific test on.
// Point the hub at a port nothing is listening on to force both spoke calls to reject.
await hub.Poll(hubSpyLogger, testHubPort, "http://localhost:11111", network.config.url);
const rejectedResponse = await sendHubRequest({ bucket: testBucket, configFile: testConfigFile }, testHubPort);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const testHubPort = 8085; // create a separate port to run this specific test on. | ||
| // Point the hub at a port nothing is listening on to force the spoke call to reject. | ||
| await hub.Poll(hubSpyLogger, testHubPort, "http://localhost:11111", network.config.url); | ||
|
|
||
| const rejectedResponse = await sendHubRequest({ bucket: testBucket, configFile: testConfigFile }, testHubPort); | ||
|
|
||
| // The failure is still reported in full, just below the level the PagerDuty transport accepts. | ||
| assert.equal(lastSpyLogLevel(hubSpy), "warn"); | ||
| assert.equal(rejectedResponse.res.statusCode, 500); | ||
| assert.isTrue(lastSpyLogIncludes(hubSpy, "Some spoke calls returned errors")); | ||
| assert.isTrue(lastSpyLogIncludes(hubSpy, "testServerlessMonitor")); | ||
| }); |
| // The PagerDuty transport only accepts `error`; `warn` still reaches Slack and GCP logging. Failures we | ||
| // can't attribute to a bot page anyway. | ||
| const failedBots = Object.keys(errorOutput?.errorOutputs ?? {}); | ||
| const pages = !failedBots.length || failedBots.some((bot) => configObject?.[bot]?.hubPageOnFailure !== false); |
There was a problem hiding this comment.
If we have concurrent failures where a "page on failure" bot is set concurrently with another bot, would that suppress the alert?
Problem
ServerlessHublogs every spoke failure aterrorwithnotificationPath: "infrastructure-error". The hub's ownPAGER_DUTY_V2_CONFIGroutes that to the shared across-bots PagerDuty service, andPagerDutyV2Transportis registered at{ level: "error" }. So any failing bot in any config file pages the on-call for the whole service, with no way to mark a bot non-critical.Tonight that meant
zion-across-relayer-sepolia— a testnet relayer, sole occupant ofacross-config-1m.json— paged 13 times in 11 hours because QuickNode's Base Sepolia endpoint started 503ing oneth_call. The bot's ownPAGER_DUTY_V2_CONFIG: { disabled: true }does nothing here: that silences the logger inside the spoke, while the page comes from the hub, a separate process with its own config.Change
A bot config may now set a top-level
hubPageOnFailure: false:When every failing bot in a run carries the flag, the hub logs the same payload at
warninstead oferror. Level is the only lever that works —PagerDutyV2Transporthas no skip path, and re-routingnotificationPathstill falls back to the defaultintegrationKey.Everything else is untouched: same message, same fields, same
notificationPath, still HTTP 500, and the(details)debug log is unchanged.Why a new key rather than reusing
PAGER_DUTY_V2_CONFIG.disabledThat flag is already set — after the
commonConfigdeep merge — on 14 bots, 11 of themHUB_CHAIN_ID: 1, includingrelayer-sweeper, bothusdc-refiller-gasless-temp*(gckms keys), and three rebalancers. Those bots disabled their own chatty error transport; nothing suggests they meant "don't page when this process hard-fails." Reusing it would drop hub paging for all of them in one commit, invisibly. It is also an env var handed to the spoke's child process, so a hub-level policy decision would ride on a value people flip for unrelated in-bot reasons.Why this is safe
Verified against this repo's winston version that transport level gating does what the fix depends on:
logger.errorlogger.warnlevel: "error")level: "info")LoggingWinston(no level)So a suppressed failure still lands in Slack and in Cloud Logging — it just stops paging. The hub's own logger is built by
createNewLogger()at leveldebug, sowarnis not filtered upstream.Defaults are fail-safe in both directions:
hubPageOnFailure !== false→ pages, exactly as today.configObject, or a thrown shape withouterrorOutputs→ pages.errorOutput instanceof Error(a fault in the hub itself) → untouched branch, always pages. See the note below.configObjectis hoisted out of thetryso thecatchcan read it; it has no other reader after thetry.hubPageOnFailureis inert everywhere else — the spoke reads onlyserverlessCommand,environmentVariablesandstrategyRunnerSpoke, and ignores unknown keys, so the flag is safe to land in a config file before or after this deploys.Known gap, deliberately not closed here
Per @md0x's review: the pre-dispatch loop awaits
_getChainId()and_getLatestBlockNumber()per bot with no guard, so one bot's dead endpoint throws anError, aborts the wholetry, and lands in the fatal branch — which pages unconditionally and dispatches no bots from that file.Consulting
hubPageOnFailurein that branch would make things worse, not better.across-config-3m.jsonpairszion-across-gasless-relayer-sepoliawithcctp-v2-finalizer,fast-finalizer,hyperliquid-finalizerand bothinventory-managers. Silencing the fatal branch because the triggering bot is testnet would let a Sepolia RPC outage stop every mainnet finalizer in that file with no page at all. A whole-run abort should always page.The right fix is the other half of that review comment — isolate the per-bot block-number fetch so a dead endpoint surfaces as that bot's own spoke error and the remaining bots still dispatch. That is an availability bug that exists today independent of paging, it changes dispatch semantics for every bot, and it wants its own tests. Tracking separately.
Tests
Two added to
test/ServerlessHub.js:does not page when every failing bot sets hubPageOnFailure: false— asserts the log drops towarnwhile the 500 and the error detail survive. Fails on master (expected 'error' to equal 'warn'), which is the check that this change is doing anything.still pages when a bot without hubPageOnFailure: false fails alongside one with it— the over-suppression guard: a flagged bot must not silence a real one. Passes before and after, by design.Full package suite green — 21 passing.
Follow-up
Landing this does not change behaviour on its own; a bot has to opt in. The sepolia relayer's
hubPageOnFailure: falsegoes inbot-configsonce this is released and the hub image is rolled. Its RPC outage is fixed separately in UMAprotocol/bot-configs#4309.