feat(0188): usage against quota on the dashboard - #227
Conversation
GET /api-tokens/api/usage reads GetUsage for the caller's key — used / remaining / reconstructed limit plus our calendar-month period boundaries — behind the portal gate, read-only by construction (no create, no attach, no delete on any path). A 60s in-process cache keyed by caller keeps refresh loops off the account-wide control-plane budget; a throttled control plane is served the last good answer with its original as_of rather than an error page. The lag wording is decided here once, for 0193 to restyle. IAM: one statement (apigateway:GET on the free plan's /usage) in 0187's standalone policy, pinned by a new CI assertion. Cold-start plan validation declined, reasoning at the grant. 24 new Rust tests (mock control plane now serves GetUsage), 9 new frontend tests.
Spec review: fetchUsage no longer reads the gate's empty 404 as 'no key' (envelope code checked); 15s usage timeout so the backend's 503 wins the race against the page's own; CI 5b hardened (suffix match, wildcard refusal, gateway-template-only); limits render in the no-key state; the lag line spells UTC, not toUTCString's GMT. Correctness review: usage failures route through describeFailure (an expired session reads the same in both sections); a successful issue evicts a cached 'no key' via a narrow UsageCache handle; a served stale answer is re-stamped so a throttle event is actually backed off; cache entries die at the month boundary; the GetUsage query ends today rather than at a future endDate no live control plane ever validated; a throttled key lookup is now assertable (throttle_list knob) and asserted. +7 Rust tests, +3 frontend tests; findings table in the task file.
…ce, flicker, malformed rows Four confirmed findings from the branch review: - the 10s deadline's 503 arm now serves the last good cached answer first — a throttle that manifests as SDK-retry latency is the same condition as a surfaced 429 and gets the same fallback - per-caller eviction epochs close the write-after-eviction race: a 'no key' snapshotted before a concurrent issue is discarded instead of cached, so the R2 fix holds under concurrency - the usage section refetches only out of the no-key state, so revealing an existing key no longer blanks rendered numbers into a loading flicker - a malformed GetUsage daily pair ([121], []) is warn-and-skipped instead of defaulting remaining to 0 and rendering a barely-used key as quota-exhausted; the mock serves raw rows so both paths are tested +5 Rust tests, +1 frontend test; findings C1-C10 dispositioned in the task file (C3 Instant-vs-frozen-Lambda accepted and noted for 0194).
karczuRF
left a comment
karczuRF
left a comment
There was a problem hiding this comment.
Code review of the usage-against-quota slice. The Rust caching layer and the throttle handling hold up well under scrutiny — I specifically checked and cleared: into_service_error() on a non-service error does not panic in aws-smithy-runtime-api 1.12.3 (it builds Unhandled), no Mutex guard is held across an .await anywhere in the cache, the CI /usage" suffix match does not collide with 0187's .../usageplans/ + /keys ARN, and {proxy+} does route /usage since PORTAL_API_METHODS includes GET.
Five findings below, all low severity — clustered on the frontend and on one honesty gap.
Reviewed as the base of a two-PR stack; #230 is built on this branch, so any fix here needs a rebase there before it merges.
| const limits = (resetsAt?: string) => ( | ||
| <> | ||
| <p> | ||
| Rate limit: <strong>1</strong> request per second. |
There was a problem hiding this comment.
Hardcoded rate limit can silently go stale.
Rate limit: <strong>1</strong> request per second is a literal, but the real limit is config.pricingApiFreePlanRateLimit (infra/envs/production.json:6) — a per-env config value the gateway stack feeds to addUsagePlan.
Raise it to 5 in production.json and deploy: the plan enforces 5/s while the dashboard — the section whose whole stated theme is rendering honestly — keeps stating 1/s, with no test or CI check tying the two together.
Every other number on this panel comes from the backend. This is the only one that can drift.
| } | ||
| throw new PortalApiError(`${url} answered 404`, 404); | ||
| } | ||
| if (!response.ok) { |
There was a problem hiding this comment.
The backend's error message is discarded, defeating the timeout rationale.
For every non-404 status the error envelope is dropped and only answered ${status} is thrown, so the backend's usage_unavailable message ("AWS is rate-limiting the usage lookup right now; try again in a moment" / "taking too long to answer") never reaches the page. The visitor sees Could not load your usage: /api-tokens/api/usage answered 503.
This contradicts the rationale written for USAGE_TIMEOUT_MS at line 100 — "the page would report its own timeout instead of the backend's more useful answer; 15s leaves the answer time to arrive." The extra 5s is spent waiting for a message that is then thrown away.
Concrete case: a throttle event with nothing cached — the one situation the backend authored a distinct 503 for — renders as a bare status code.
| // blanking it into a loading flicker for an identical body would make the | ||
| // press look like it broke something. | ||
| useEffect(() => { | ||
| if (keyOnScreen && viewState.current === 'no-key') load(); |
There was a problem hiding this comment.
Refetch-after-issue can never fire if the mount-time fetch is still in flight.
The refetch fires only on the false -> true transition of keyOnScreen and only when viewState.current === 'no-key'.
Press "Get my key" while the mount-time usage fetch is still in flight (view is 'loading') and the effect is a no-op. The in-flight request then resolves no_key — it was issued before the key existed — and settles the view to 'no-key', but the effect's deps (keyOnScreen, load) no longer change, so no refetch ever fires. A second press can't retrigger it either: setKeyOnScreen(true) on an already-true state bails out of re-render.
The section stays on "Your key is new — usage figures appear here with a delay" until the user finds the Refresh button.
The backend half of this race is handled correctly (remember discards the stale NoKey via the epoch guard), so a manual refresh does return real data.
| }; | ||
|
|
||
| let today = Utc::now().date_naive(); | ||
| let period = current_period(today); |
There was a problem hiding this comment.
The period mismatch corrupts the reconstructed limit, not just the label.
limit is reconstructed as used + remaining, where used is summed over [period.start (our calendar-1st), today] but remaining is the last day's balance.
The module docs call an AWS/our-rule period mismatch "a UX wrinkle to word around, not a correctness bug in this module." But if AWS's MONTH quota rolls at any instant other than the calendar 1st 00:00 UTC — which ADR 0010 correction #2 says is undocumented — the sum spans two AWS periods while remaining reflects only the current one. The dashboard then renders a fabricated "Monthly limit": e.g. 150000 against a 100000 plan.
Worth either bounding the query start at the key's first observed row, or noting the limit-reconstruction exposure explicitly alongside the existing wrinkle note.
| /// callers seen in the last [`STALE_KEEP`], which for this portal is a small | ||
| /// number — and a warm Lambda container is the only place they live long | ||
| /// enough to matter. | ||
| fn remember(state: &UsageState, sub: &str, answer: CachedAnswer, epoch: u64) { |
There was a problem hiding this comment.
Epoch guard can discard a legitimate NoKey when a mark is pruned mid-lookup.
remember's guard compares against cache.epochs.get(sub).unwrap_or(0), but epoch marks are pruned at STALE_KEEP (15 min). If a caller's mark was bumped (nonzero) more than 15 minutes ago and any other request's remember prunes it during this lookup's ~10s window, the comparison becomes 0 != <stale nonzero epoch> and a perfectly legitimate NoKey is discarded instead of cached — costing a GetApiKeys on every subsequent load.
Only reachable for a caller who issued a key and no longer has one, so it's rare. The cleaner invariant is to treat "no mark" as "matches any epoch".
Related and equally minor: invalidate_no_key inserts into epochs without pruning, and remember is the only pruner — so a warm container where the usage route consistently errors accumulates one mark per key issuance.
The GetUsage query starts on our calendar 1st while `remaining` is the last day's balance. If AWS's MONTH quota rolls at any other instant -- undocumented, ADR 0010 correction #2 -- the range spans two of its periods: `used` sums both, `remaining` describes one, and the reconstructed `used + remaining` renders a quota no plan has. `remaining` is a running balance, so a day whose value RISES can only be a reset. `summarize_days` counts from it, which puts both figures in one period whatever instant that period began, and warns when it sees one -- the only evidence this system can produce about the instant ADR 0010 is still open on. Review finding O4 (PR #227). The review read this as a corrupted `limit`; `used` is wrong first and `limit` is its consequence.
The guard read `unwrap_or(0)`, so a mark pruned at STALE_KEEP while a lookup was in flight compared as a moved epoch and threw away a legitimate `NoKey` -- buying a GetApiKeys on every load after it, for a caller who genuinely has no key. Absence is safe to trust rather than merely likelier to be right: `invalidate_no_key` stamps `bumped_at` as it bumps, so any eviction inside the window leaves a mark too fresh to prune. No mark at `remember` time therefore proves no eviction happened. `invalidate_no_key` now prunes too. It was a writer to `epochs` with no pruner behind it on the paths where `remember` is never reached -- a throttled or erroring container accumulated one mark per key issuance. Review finding O5 (PR #227), correcting the epoch guard added for C2.
`Rate limit: 1 request per second` was a literal on the one panel whose stated theme is rendering honestly, while the figure actually enforced is `pricingApiFreePlanRateLimit` -- a per-env config value the gateway stack hands to `addUsagePlan`. Raise it in production.json, deploy, and the dashboard would keep stating the old number with nothing tying the two together. It travels the same value: compute-stack passes PORTAL_RATE_LIMIT from that config key, AppConfig reads it, /config serves it as `rate_limit_per_second`, and the page renders that. Not read back from GetUsagePlan, which would cost a control-plane grant this slice deliberately does not take. Absent means the line is omitted, never defaulted -- a fallback figure here would be the same silent staleness one layer down, wearing the authority of a number. Review finding O1 (PR #227), which the third pass had deferred to 0193 or 0194 with /config named as the likely home.
Every non-404 dropped the error envelope and threw `answered ${status}`,
so `usage_unavailable`'s authored message -- "AWS is rate-limiting the
usage lookup right now; try again in a moment" -- never reached the
page. A throttle with nothing cached, the one case the backend wrote a
distinct 503 for, rendered as a bare status code.
It also made USAGE_TIMEOUT_MS pointless: the extra five seconds over the
probe timeout exist so the backend's more useful answer has time to
arrive, and it was then discarded on arrival.
`readEnvelope` reads the body once (a Response body is a stream, and
fetchUsage needs the same read to recognise `no_key`), `failureMessage`
prefers its message and falls back to the URL and status when there is
none -- the gate's empty 404, or anything answered on the backend's
behalf. Applied to getJson and issueKey too: one wording per cause is
R1's rule, extended to the failures the backend words itself.
Review finding O2 (PR #227).
The refetch fired on the `keyOnScreen` transition and read the view state through a ref. Press "Get my key" while the mount-time usage fetch is still in flight and the transition happens with the view still 'loading', so the effect had nothing to refetch out of -- and never ran again, because its dependencies had already settled. The in-flight request, issued before the key existed, then resolved `no_key` and the section sat on "your key is new" until the visitor found Refresh. A second press could not help: setKeyOnScreen(true) on an already-true state changes no dependency. The effect now watches `view.state`, which is the honest dependency, and a ref latch makes it fire at most once per mount. That is what the ref read was really buying: the backend can legitimately keep answering `no_key` while its own cache catches up, so "no-key -> load -> no-key" would otherwise re-trigger itself forever. Both properties are tested. Review finding O3 (PR #227), correcting the guard added for C4.
Five findings from PR #227, all fixed. Two of them corrected earlier passes of this same task -- C2's epoch guard and C4's refetch guard were each right about the race they were written for and wrong at one edge -- which is the part worth having written down. Also strikes the third pass's deferral of the rate-limit literal: it guessed /config as the number's home, and that is where it went.
…ship-and-account-age #227 put 0188 on develop, so this branch retargets there — and develop had moved eleven commits further (0190, 0204, 0213, 0120, 0135) while this slice was in review. One conflict, in 0189's own task file: develop carries the stub as it stood at `chore(lore-0189): activate task`, this branch carries the implementation record written on top of it. Ours wholesale — everything develop's copy has is a subset, apart from the older parameter-table wording that decision #21 deliberately corrected (`stellar_test` as a guild id was the trap that refused every visitor). Workspace 610 passed, portal 76 passed, clippy clean, fmt/format, synth and the openapi checks all green.
rustc 1.98.0, released 2026-08-18, began passing `-Wl,--fix-cortex-a53-843419` on aarch64-unknown-linux-gnu. Zig's linker rejects the argument outright, so every aarch64 link fails and the `Build Lambda bootstraps` step dies on the first crate that links — `crc-fast`, pulled in by aws-smithy-checksums. Nothing in this repo caused it: PR #227 passed on 1.97.1 and PR #230 failed on 1.98.0 with an identical Cargo.lock and the same cargo-lambda 1.9.1 / zig 0.16.0 pair. `develop` did not show it because its rust job has not run since 22 July. cargo-zigbuild filters the argument since v0.23.0 (rust-cross/ cargo-zigbuild #451, fixed by #452), but cargo-lambda 1.9.1 still vendors 0.20.1 — so upgrading cargo-lambda cannot reach the fix today. Both pins come off together once a cargo-lambda release carries zigbuild >= 0.23. cargo-lambda is pinned as well because `pip3 install cargo-lambda` resolves at run time, which is how the vendored zigbuild version — the half that actually carries the fix — could change under a green build without a commit. Considered and rejected: `cargo lambda build --compiler cargo` on the native ARM runner. ubuntu-24.04 ships glibc 2.39 against provided.al2023's 2.34, so the failure would move from CI to the deployed function.
Both shipped to `develop` today: 0188 in PR #227 (`a76d8a9`), 0189 in PR #230 (`99bca3a`, approved by Oskar Karcz). Workspace ends at 558 Rust tests and 61 portal tests, 0 failures. 0188 closes seven of eight criteria; "N requests move the number" cannot be closed from a keyboard and waits on the deploy with 0187's live curl. Carried forward from its four review passes: the cached "no key" that survived the issue falsifying it, the eviction race that followed, and the running-balance reset detection that keeps `used` and `remaining` in one AWS period — the only evidence this system can produce about the instant ADR 0010 correction #2 is open on. 0189 closes all twelve criteria in code, and deliberately does not close Step 0: the five Discord measurements are operator-owned, the tables keep a dated deferral, and no result was invented. The two findings worth carrying are written into the task — `prompt=none` asserted by three comments and sent by no code, and one `is_snowflake` now guarding both the cold-start probe and the member URL. Still operator-owned before production: the Developer Portal scope, the two SSM seeds, and 0179 step 4. Nothing spawned; every follow-up already has an owner.
2 participants
Summary
GET /api-tokens/api/usage(newportal/usage/module): the signed-in caller's used / remaining / reconstructed limit fromGetUsage, plus period boundaries rendered as our calendar-month UTC rule (ADR 0010 correction Feat/lore 0001 dump swap events #2 — AWS documents no reset instant). Behind 0183's gate, session-only, and read-only by construction: a dashboard load can never create, attach or delete a keyas_of, so the page dates staleness honestly instead of erroringapigateway:GETon/usageplans/{planId}/usage, added to 0187's standalone policy; pinned by a new CI assertion (exists, exactly one, no wildcard, gateway stack only). No gateway route change —{proxy+}already covers/usageGetUsage, wire-fieldvalues, with throttle/failure/pagination knobs) + 12 frontend tests; two independent review passes with all findings fixed and the trail recorded in the task file