[pull] main from firecrawl:main - #413
Merged
Merged
Conversation
…r our own stalls (#4514) * fix(billing): read firebill's "I do not know", and stop blaming it for our own stalls Two logging/metric corrections in the firebill client. No existing `outcome` value changes, so every alert reading them counts the same thing. **Read `ambiguous` explicitly.** firebill answers a publish whose broker confirm timed out with `504 {"success": false, "ambiguous": true}` rather than the plain `{"success": false}` it sends when it is certain it took nothing. Over eight days ~2,100 tracks arrived as the latter and were keyed to `refused` — the outcome that means "this usage is gone" — while every one of those events had in fact been committed and settled. `firebillAttempt` now reads the field first and the 504 second, and only a plain `success: false` is still `refused`. **Stop blaming firebill for client-side stalls.** `firebill track attempt failed — firebill may be unavailable` and `firebill check unavailable — request threw` fired ~1,800 times per 30h against a firebill whose server-side p99 is 73ms; the attached errors were almost all our own 5s `AbortSignal.timeout`, plus a few ECONNREFUSED/ECONNRESET. The track, check, lock and finalize paths now separate a request that never completed from a firebill that answered, name the status when it did, and carry the error's `name`/`code` — unwrapped from the bare `TypeError: fetch failed` undici reports it inside. `firebillTrackTotal` and `firebillCheckTotal` gain a low-cardinality `cause` label (`none`, `timeout`, `connection`, `non_ok`, `refused`, `ambiguous`) that splits the existing outcomes without changing them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(billing): keep the alerted firebill series intact, and stop reading an unreadable answer as a refusal Review follow-ups. The metrics one is the load-bearing change. **Do not put `cause` on the counters the alerts read.** `increase()` evaluates per series, so `increase(firecrawl_firebill_check_total{outcome="unavailable"} [15m]) > 5` — the live FirebillCheckUnavailable rule — would have started comparing 5 against each cause separately: two causes at 4 apiece is 8 unanswered credit checks nobody is paged about. `firebillTrackTotal` and `firebillCheckTotal` are restored byte-for-byte, and the detail moves to a new `firecrawl_firebill_failure_cause_total{operation, cause}`, incremented beside them on every failure and never on success. **Only `success: false` is a refusal.** An empty, malformed, or `success`-less 200 was being counted as one — logging usage as lost when firebill may well have taken the event and said so in a shape we could not parse. Those are now `ambiguous` with cause `unusable`. Same correction on `/v1/check`, where `refused` was labelling a missing `allowed`. **A body that never arrived is not an answer.** `readJson` swallowed every throw from `response.json()`, including a body timeout or a connection lost after the headers — reporting a transport failure as something firebill said. Only a parse error is caught now; everything else re-throws to `classifyTransportError`. Conversely the lock and finalize paths parsed inside their broad `try`, so a malformed 200 was logged as a connection failure; both now read through `readJson` and report an unusable answer. **Static log messages.** Every message added in this branch is a literal, with the status and the error's name and code in the structured context. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )