Fix BL-16576 stop reporting missing API endpoints that external tools legitimately probe - #8126
Fix BL-16576 stop reporting missing API endpoints that external tools legitimately probe#8126JohnThomson wants to merge 2 commits into
Conversation
…racing client calls https://issues.bloomlibrary.org/youtrack/issue/BL-16576 Sentry issue https://bloom-app.sentry.io/issues/7254113070/ (1115 events, 20 users over 90 days) turned out to be three unrelated endpoints, none of which is a Bloom programming error, all reported to the user as one. ShouldReportMissingApiEndpoint now suppresses the user-visible report (still returning 404) for: - edit/pageControls/cleanup. Its registration was deliberately removed in BL-15934 and nothing in the repo calls it, but stale clients in the field still POST it. Nothing we or the user can do about that. - The API surface we expose to other processes: common/instanceInfo and anything under external/. BloomBridge and our automation scripts scan for a running Bloom and then discover what it supports by asking, treating a non-2xx answer as "this Bloom is too old for that" (see the comment on getCollectionLanguages in BloomBridge's notifyBloom.ts). A 404 there is normal version negotiation with a client we do not control, so the user -- who may not even know a tool is talking to Bloom -- should not get a toast/modal about it. These are logged instead, so a silently-no-op external integration is still diagnosable. branding/image, the third endpoint, was already suppressed by BL-16300; no change was needed. Note on the card's diagnosis: the common/instanceInfo reports are NOT a startup race. That endpoint is application-level -- ApplicationContainer's constructor registers CommonApi and then calls RecordApplicationLevelHandlers, and the server does not start listening until after that -- so there is no window in which Bloom answers HTTP without it. It is version skew: the endpoint was only added 2026-03-12 (27429c5, master -> 6.4+, not in Version6.3), and the Sentry events span 6.2.x-6.4.x. Guarding the caller cannot fix it, because BloomBridge's capability probe IS the 404 and there is no older version endpoint to gate on. So the fix has to be here, and it is forward-only: it stops new builds from nagging users as the BloomBridge contract keeps growing, but cannot retire the existing events from already-released builds. Tests: added MissingPageControlsCleanupApiEndpoint_DoesNotReportNonFatalProblem and MissingExternalToolApiEndpoint_DoesNotReportNonFatalProblem (two cases: common/instanceInfo and an external/ endpoint) alongside the existing BL-16300 pair. Ran the full BloomServerTests fixture via build/agent-dotnet.sh: 49 passed, 0 failed. MissingNonLegacyApiEndpoint_ReportsNonFatalProblem still passes, confirming the reporting path is intact and the new tests are not passing falsely. csharpier reports both changed files already formatted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Opus 5] Consulted Devin on 2026-07-29 up to commit One informational item, not mirrored as a thread because it is the trade-off this PR already documents in both the description and the code comment: suppressing the whole Other reviewers: Local verification: full |
Sentry issue F6Z / 7254113070 — 1115 events, 20 users over 90 days — turned out to be three unrelated endpoints, none of them a Bloom programming error, all reported to the user as one alarming "Cannot Find API Endpoint" toast.
ShouldReportMissingApiEndpointnow suppresses the user-visible report (still returning 404, and logging) for:edit/pageControls/cleanup— its registration was deliberately removed in BL-15934, and there is no caller left anywhere in the repo (.ts/.tsx/.cs/.js/.html/.pugall searched). Stale clients in the field still POST it, and there is nothing we or the user can do about that.common/instanceInfoand anything underexternal/. BloomBridge and our automation scripts scan for a running Bloom and then discover what it supports by asking, treating a non-2xx as "this Bloom is too old for that". A 404 there is normal version negotiation with a client we don't control, so the user — who may not even know a tool is talking to Bloom — should not get a toast about it. These are logged instead, so a silently no-op integration is still diagnosable.branding/image, the third endpoint, was already suppressed by BL-16300; no change needed.The card's diagnosis was wrong about
common/instanceInfoWorth recording, since it changes what the fix can achieve. The card called it a startup race and said to guard the caller. Neither holds:
ApplicationContainer's constructor registersCommonApithen callsRecordApplicationLevelHandlers(), and the server does not start listening until after that. There is no window where Bloom answers HTTP without it.27429c581b), when master was6.4.0.0; it is not in Version6.3 at all, and the events span 6.2.x–6.4.x.Reproduced end to end against an installed Bloom 6.3.2.0: one
GET /bloom/api/common/instanceInforeturned 404 and logged— the card's exact message, from a single ordinary discovery request.
Scope: this change is forward-only
Nothing shipped now can alter the 6.2/6.3/6.4 builds already installed, so this cannot retire the existing Sentry events — expect them until those releases age out. Its value is forward protection as the external-tool contract keeps growing.
The half that actually stops what users see today is caller-side, in BloomBridge: BloomBooks/BloomBridge#14, which determines the Bloom version from Windows before opening any connection and refuses to knock on anything older than 6.5.
Trade-off worth a reviewer's eye
Suppressing the whole
external/namespace means a typo in some future external endpoint of ours won't self-report. I judged that a good trade — nothing inside Bloom calls these, so such a mistake surfaces immediately in the external tool — and said so in the code comment. Narrowing it to just today's endpoint names is the alternative.Tests
Added
MissingPageControlsCleanupApiEndpoint_DoesNotReportNonFatalProblemandMissingExternalToolApiEndpoint_DoesNotReportNonFatalProblem(two cases:common/instanceInfoand anexternal/endpoint), alongside the existing BL-16300 pair. FullBloomServerTestsfixture: 49 passed, 0 failed.MissingNonLegacyApiEndpoint_ReportsNonFatalProblemstill passes, which is the guard that the reporting path is intact and the new suppression tests aren't passing for the wrong reason.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16576
🤖 Generated with Claude Code
Devin review
This change is