Fall back to text-only posts and collapse Telegram polling noise - #66
Merged
Merged
Conversation
TELEGRAM_IMAGES_DIR is unset in the deployed environment, so every video-bearing notification builds the literal path "undefined/<file>.mp4". node-telegram-bot-api reinterprets a path that is not a readable file as a URL, Telegram answers "400 Bad Request: wrong HTTP URL specified", and sendMessage's catch only logs a warning — the notification is dropped. Trade, MintingUpdate, SavingUpdate, StablecoinBridgeUpdate and FrontendCodeRegistered have therefore never reached subscribers, while text-only alerts kept working and made the bot look healthy. Setting the env var alone would not help: the media files are not in the image. Port the guard d-EURO#115 already uses: resolveMediaPath checks the file with existsSync and returns undefined, so sendMessage degrades to sendMessage instead of sendVideo. Separately, attach a 'polling_error' listener. Without one the library writes its own unformatted console error for every failed poll, and it retries on a fixed interval with no backoff — a nine-second Telegram gateway outage produced 27 error lines, and sustained retries earn a 429 on top of the original 502. The listener reports one line per distinct failure plus one on recovery with duration and attempt count, which turns the same outage into two lines while still surfacing an escalation from 502 to 429. Both lines are logged at warn so an outage never appears to stay open under a warn-level log configuration. No unit tests added: the repo's jest config has rootDir="src" but the sources live at the repo root, so `yarn test` finds zero tests today.
Review follow-up on the first two commits. The missing-asset notice was logged at debug, which the deployed logger never emits. Since the assets are absent from the image the fallback is the permanent state, so the condition would have become completely silent — it was visible as a warning before this branch. Report it at warn, once per distinct asset per boot. The polling signature used the raw error message compared against the previous one only, which fails in the two cases that actually occur: "Too Many Requests: retry after N" counts down and a connect failure carries a rotating gateway address, so each retry looked like a new failure, and alternating errors re-reported on every poll. Mask digits out of the signature and track the signatures already reported within the current outage. A permanently failing poll — a revoked token answers 401 forever — reported once and then stayed silent, because the recovery timer only fires once errors stop. Repeat the report every five minutes while the outage is open. Also apply the same media guard to TwitterService, which is the other caller of the shared helper and logged an upload error per post, move the outage shape into telegram.types.ts next to the other state types, and truncate the error text, which wraps the upstream response body.
Second review follow-up. The signature prefixed the error code onto a message that the library already prefixes with that same code, so the prefix discriminated nothing. Use the message alone. The signature set had no upper bound: a message carrying a per-attempt token that survives digit masking - a request id in an upstream error body reaches the parse-error branch verbatim - produced a fresh signature per poll, which defeated the collapsing and grew the set without limit. Cap it, past which only the periodic report remains. A replay of 2000 such polls now yields 22 lines and 20 retained signatures instead of 2000 of each. An error arriving more than the grace period after the previous one opened and closed its own outage, so an isolated blip cost two lines where it used to cost one. Skip the closing line for a single attempt. The closing line said "recovered", but nothing probes the poll - the grace period only establishes that no further error arrived, and a request that never settles would look the same. Say what is actually known instead.
Author
|
This went through 12 review passes before it was clean. Recording what changed, since several rounds altered real behaviour: Code fixes (rounds 1–3)
Description fixes (rounds 4–12) — all cases where the description claimed more than the code delivers:
Every replay figure in the description was re-derived against the built code rather than estimated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent problems in the social-media notification path, both visible in the deployed mainnet instance's logs.
1. Video-bearing notifications are silently dropped
CONFIG.telegram.imagesDircomes fromTELEGRAM_IMAGES_DIR, and the notification assets are not in this repo at all, so every video-bearing message builds a path that is not a readable file — literallyundefined/<file>.mp4when the variable is unset.node-telegram-bot-apireinterprets a path that is not a readable file as a URL, Telegram answers400 Bad Request: wrong HTTP URL specified, andsendMessage's catch only logs a warning — the notification is dropped, with no text-only fallback.Affected message types:
Trade,MintingUpdate,SavingUpdate,StablecoinBridgeUpdate,FrontendCodeRegistered. Plain-text alerts (minter, leadrate, position) keep working, which is why the bot looks healthy while a whole class of notifications never arrives. The failures go back at least to 2026-06-30 and only fire when there is on-chain activity, which is why they appear in clusters.Setting the env var alone would not fix it — the media files are not present in the image at all.
This ports the guard d-EURO/api#115 already uses:
resolveMediaPathchecks the file withexistsSyncand returnsundefined, so the send degrades to text-only instead ofsendVideo.TwitterServiceis the other caller of that shared helper and had the same shape — it logged an upload error per post before falling back — so it is included.A missing asset is reported once per distinct path per boot, at
warn. It is a deployment defect that never heals on its own, so it has to stay visible; with the assets absent this is up to five lines per service per boot, and it goes to zero once the assets ship.2. Telegram gateway outages produce a burst of raw error lines
Telegram's gateway answers
502/504during its own restarts. Two library behaviours turn that into a log burst:polling_errorlistener attached,node-telegram-bot-apifalls back to its ownconsole.error('error: [polling_error] %j', error)(src/telegramPolling.js) — unformatted, at error level, bypassing the application logger entirely.retry after. This bot's worst retained window produced 69 error lines over ~18 minutes, one failed poll roughly every 16 seconds, and a short gateway blip produces around three lines per second. Some bursts also carry429 Too Many Requests: retry after N; in every retained instance the 429 opens the burst, ahead of the502run — and because the loop ignoresretry after, it answers aretry after 5with seven 429s in a row, spaced about 324 ms apart, before Telegram switches to 502s.The listener reports one line per distinct failure, repeats an outage that never clears every five minutes, and closes it with one line. Digit runs are masked out of the failure signature so a counting-down
retry after Nand a rotating gateway address collapse instead of looking new each time; the number of distinct signatures per outage is capped so a per-attempt token in an upstream error body cannot defeat either the collapsing or the memory bound.Replaying real failure shapes against the built code:
Sample output for the 429-led burst row above, replayed from that burst's real timestamps:
Below the two limits noted next, every distinct failure is reported on arrival — including a 429 and a 502 within the same burst — and a permanently failing poll keeps reporting. Two mechanisms deliberately bound the worst case rather than reproducing it, and both cost information. The error text is truncated to the first
MAX_POLLING_ERROR_LENGTHcharacters before it is both logged and reduced to a signature, so anything past that never reaches the log, and failures differing only beyond it collapse into one. And onceMAX_POLLING_SIGNATURESdistinct failures have been reported within a single outage, a further new failure is no longer reported on arrival: from then on the outage emits at most one line per report interval, carrying whichever failure is in flight when that report falls due — so a new failure that has been superseded by then, or an outage that clears first, is never shown. Both log lines — the onset and the closing one — are atwarnon purpose: an outage whose end is invisible under a warn-level log configuration is worse than one extra line. The closing line deliberately says errors stopped rather than recovered — the grace period only establishes that no further error arrived, it does not probe the poll.Note that no inbound updates are lost during these outages —
getUpdatesdoes not advance its offset on failure, so Telegram re-delivers on the next successful poll. This change is about how those failures are logged. The retry cadence itself is unchanged — no backoff is added andretry afteris still ignored, so the underlying storm, including the 429 bursts described above, still happens; it just stops being reprinted line by line. Message-delivery correctness is unaffected.The polling listener is proposed for the sibling bot in d-EURO/api#127 with an identical implementation.
Verification
yarn buildclean,eslintandprettier --checkclean on all four touched files.dist/for each scenario above.rootDir="src"but the sources live at the repo root, soyarn testaborts before test discovery (Validation Error: Directory .../src in the rootDir option was not found) and no test can run today. Reworking the test topology is out of scope, matching the precedent in Handle missing social-media tokens and assets gracefully d-EURO/api#115.