Skip to content

fix(usage): surface active-provider fetch errors instead of dropping them - #759

Merged
junhoyeo merged 1 commit into
mainfrom
fix/usage-surface-provider-errors
Jun 22, 2026
Merged

fix(usage): surface active-provider fetch errors instead of dropping them#759
junhoyeo merged 1 commit into
mainfrom
fix/usage-surface-provider-errors

Conversation

@junhoyeo

@junhoyeo junhoyeo commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Problem

fetch_all in crates/tokscale-cli/src/commands/usage/mod.rs spawned each active provider as fetch.call().ok() and collapsed the results with filter_map(|h| h.join().ok().flatten()).flatten(). Any provider that returned Err was silently dropped.

The clearest failure case is Sakana: sakana::fetch deliberately builds a helpful "Sakana session expired or invalid. Refresh SAKANA_SESSION_COOKIE…" error when the session cookie is stale. But because has_credentials() returns true (a cookie exists, it's just expired), the provider is considered active — yet it just disappeared from the output with no message. This contradicts both docs/providers/sakana.md and sakana::fetch's own carefully-worded error path. The same silent-drop affected every usage provider.

Root cause

Errors were discarded at the join/merge step (.ok() + filter_map), with no channel to carry them back to the user.

Fix

  • Added fetch_all_with_errors() -> (Vec<UsageOutput>, Vec<ProviderError>). Each active provider's thread now returns (name, Result<...>); successes and errors are split by a new pure helper partition_results.
  • pub fn run now calls fetch_all_with_errors and, in non-JSON mode only, prints one concise line per failing provider to stderr: "Sakana: <error> — skipped".
  • JSON mode emits no warnings — stdout stays pure JSON and nothing is written to stderr, so --json consumers that also read stderr are not corrupted. (Note: pre-existing unconditional eprintln!s in minimax_tokenplan.rs already violate this, but that's out of scope here.)
  • fetch_all is retained as a thin error-discarding wrapper (fetch_all_with_errors().0), so the TUI dashboard caller (tui/app.rs) is unchanged. No UsageOutput struct change was needed.

Approach / tradeoffs

I chose the least-invasive option from the brief: collect per-provider errors and surface them as a one-line stderr warning in non-JSON mode. Rejected a sweeping UsageOutput/struct redesign (carrying an error variant through serialization and the TUI rendering path) as unwarranted for the goal of making failures visible. The TUI continues to use fetch_all and ignores errors for now; fetch_all_with_errors is public so the TUI can adopt error display later without a signature change.

Tests

  • Refactored the join/collect logic into a pure partition_results and added two regression tests:
    • partition_results_surfaces_provider_errors_instead_of_dropping_them — a failing Sakana provider's error (including the SAKANA_SESSION_COOKIE guidance) is surfaced, not discarded, while successful providers (including a Multi provider's several outputs) are preserved in order. This fails against the old drop-everything behavior.
    • partition_results_reports_no_errors_when_all_succeed — success path unchanged.
  • cargo test -p tokscale-cli and cargo clippy -p tokscale-cli --tests are clean for this file (remaining clippy warnings are pre-existing in report.rs, untouched here).

Residual concerns

  • Provider thread panics are skipped (join error has no useful message); this matches prior behavior of not crashing the command.
  • No real network/auth round-trip is exercised — only the pure partition logic is unit-tested.

🤖 Generated with Claude Code


Summary by cubic

Surface fetch errors from active usage providers in the usage command instead of silently dropping them, so users know why a provider was skipped. Non-JSON runs now print one concise stderr warning per failing provider; JSON output is unchanged.

  • Bug Fixes
    • Fixed silent drops by collecting per-provider Err instead of discarding them at join time.
    • Added fetch_all_with_errors() returning (Vec<UsageOutput>, Vec<ProviderError>) and a pure partition_results to split successes/errors.
    • Updated CLI run to print "Provider: — skipped" to stderr in non-JSON mode only; --json emits clean JSON with no warnings.
    • Kept fetch_all() as a thin, error-discarding wrapper so the TUI remains unchanged.
    • Added regression tests to verify errors are surfaced and success paths are intact.

Written for commit bcd1785. Summary will update on new commits.

Review in cubic

…them

`fetch_all` spawned each active provider as `fetch.call().ok()` and merged
results with `filter_map(|h| h.join().ok().flatten()).flatten()`, so any
provider that returned `Err` was silently discarded. The most visible case
is sakana::fetch building a "refresh SAKANA_SESSION_COOKIE" auth error: the
provider's `has_credentials()` still reports it active, yet it just vanished
from output — contradicting docs/providers/sakana.md and sakana's own error
path.

Root cause: errors were thrown away at the join/merge step, with no channel
to carry them back to the user.

Fix: add `fetch_all_with_errors()` returning both successful outputs and a
`Vec<ProviderError>` (provider name + formatted message). The merge logic is
extracted into a pure, unit-testable `partition_results`. The CLI `run` uses
the new function and prints one concise warning per failing provider to
stderr in non-JSON mode ("Sakana: <error> — skipped"). JSON mode emits no
warnings so `--json` consumers reading stderr are not corrupted. `fetch_all`
is kept as a thin error-discarding wrapper so the TUI caller is unchanged.

Confidence: high
Scope-risk: narrow
Not-tested: real provider network/auth round-trip (covered only by the pure partition test)
@vercel

vercel Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
tokscale Ignored Ignored Jun 22, 2026 8:21am

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@junhoyeo
junhoyeo merged commit dc9e95a into main Jun 22, 2026
14 of 15 checks passed
makoMakoGo added a commit to makoMakoGo/tokscale that referenced this pull request Jun 23, 2026
ported from upstream junhoyeo#728
ported from upstream junhoyeo#757
ported from upstream junhoyeo#759
ported from upstream junhoyeo#760
t1000040 pushed a commit to tmobi-internal/tokscale that referenced this pull request Jun 30, 2026
…them (junhoyeo#759)

`fetch_all` spawned each active provider as `fetch.call().ok()` and merged
results with `filter_map(|h| h.join().ok().flatten()).flatten()`, so any
provider that returned `Err` was silently discarded. The most visible case
is sakana::fetch building a "refresh SAKANA_SESSION_COOKIE" auth error: the
provider's `has_credentials()` still reports it active, yet it just vanished
from output — contradicting docs/providers/sakana.md and sakana's own error
path.

Root cause: errors were thrown away at the join/merge step, with no channel
to carry them back to the user.

Fix: add `fetch_all_with_errors()` returning both successful outputs and a
`Vec<ProviderError>` (provider name + formatted message). The merge logic is
extracted into a pure, unit-testable `partition_results`. The CLI `run` uses
the new function and prints one concise warning per failing provider to
stderr in non-JSON mode ("Sakana: <error> — skipped"). JSON mode emits no
warnings so `--json` consumers reading stderr are not corrupted. `fetch_all`
is kept as a thin error-discarding wrapper so the TUI caller is unchanged.

Confidence: high
Scope-risk: narrow
Not-tested: real provider network/auth round-trip (covered only by the pure partition test)
@junhoyeo
junhoyeo deleted the fix/usage-surface-provider-errors branch July 13, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant