Skip to content

Log the reason a token exchange fails in verbose output - #8216

Merged
wes-shaw merged 8 commits into
Shopify:mainfrom
wes-shaw:automation-token-identity-error
Aug 5, 2026
Merged

Log the reason a token exchange fails in verbose output#8216
wes-shaw merged 8 commits into
Shopify:mainfrom
wes-shaw:automation-token-identity-error

Conversation

@wes-shaw

@wes-shaw wes-shaw commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

When an app automation token exchange fails, the CLI prints The custom token provided can't be used for the App Management API. with nothing about why. Identity's response carries an OAuth error code and an error_description that provides more context but the CLI discards both. Anyone debugging a CI failure has to guess, or find someone with access to Identity's logs.

All of these cases resolve to the same user action (create a fresh token), so the default error output does not need to change. The detail belongs in --verbose logs, where app developers and support can read it without Identity-side access. That is the full scope of this PR: default output stays byte-identical to main.

WHAT is this pull request doing?

Adds two debug lines, both visible only with --verbose:

  1. When Identity returns an OAuth failure body, tokenRequest logs it: Token request to Identity failed with status 400: invalid_grant - <description>. The description is flattened to a single line and capped at 200 characters.
  2. When the app automation token exchange throws with no OAuth body at all (for example, an unreachable Identity host), the catch block logs the caught error's message, so --verbose always carries a reason.

It also fixes two pre-existing bugs on main that surfaced while testing this path. Their fixes are interleaved with the logging commits, so they are included here (with a note in each changeset) rather than split into a separate PR:

  • Device authorization polling looped forever when Identity returned an error code outside the known set; it now fails with an AbortError.
  • An OAuth failure body with a missing or non-string error field produced AbortError(undefined); it now normalizes to unknown_error.

One suggestion from the internal discussion was a Re-run with --verbose pointer in the default error text. It is deliberately left out to keep the default output unchanged; happy to add it in a follow-up if reviewers want it.

How to test your changes?

  • Unit tests: from packages/cli-kit, run pnpm exec vitest run src/private/node/session/exchange.test.ts src/private/node/session/device-authorization.test.ts (46 tests cover the failure paths above).
  • Manual: set SHOPIFY_APP_AUTOMATION_TOKEN to an expired or revoked token and run shopify app deploy --verbose. The debug output includes the Token request to Identity failed with status 400: ... line, and the final error banner is unchanged from main.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

🤖 Generated with Claude Code

@wes-shaw
wes-shaw requested review from a team as code owners July 30, 2026 18:43
Comment thread .changeset/device-auth-unknown-error-hang.md Outdated
Comment thread packages/cli-kit/src/private/node/session/device-authorization.ts Outdated
Comment thread packages/cli-kit/src/private/node/session/exchange.ts Outdated
Comment thread packages/cli-kit/src/private/node/session/exchange.ts Outdated
Comment thread packages/cli-kit/src/private/node/session/exchange.ts Outdated
Comment thread packages/cli-kit/src/private/node/session/exchange.ts
wes-shaw and others added 6 commits August 3, 2026 14:39
…anged

Every app automation token exchange failure collapsed into a single fixed
message, and the OAuth `error`/`error_description` returned by Identity were
discarded before reaching it, so a revoked token, an expired token and an
unreachable Identity were indistinguishable. Verbose output didn't help either:
it printed the response headers for that call and no status or body.

Keep the existing sentence as the headline and append the upstream reason when
there is one, and log the status plus the OAuth error fields of a failed token
request under `--verbose`. Only failed responses are logged, and only the
`error`/`error_description` fields, since a successful body carries the access
token.
The fields of a failed token response aren't guaranteed: the responder
isn't always Identity, since proxies and gateways can answer with
arbitrary JSON. A body without an `error` field crashed the error
constructor before any message was shown, and `error_description` flowed
into the terminal unbounded.

Guard both fields once, where the response is parsed: `error` falls back
to `unknown_error`, and `error_description` is flattened to a single line
and capped at 200 characters. Every consumer downstream now gets the
shape the type declares.

Also widen the changeset wording: the shared error handler enriches
interactive token errors too, not only automation-token ones.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Map any code outside IdentityDeviceError to unknown_failure at the
exchange boundary instead of casting, and add a default branch to the
poll switch so an unexpected code rejects rather than leaving the
promise unsettled. Also apply one shared flatten-and-truncate rule to
both the Identity description and the appended Reason message.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review direction from the proposal thread: every token failure resolves
to the same user action, so the default error stays generic and the
reason lives in the debug log only.

The debug line in tokenRequest covers OAuth rejections. The automation
catch now logs the caught error too, which covers failures that produce
no OAuth body, such as the authentication service being unreachable.
The enrichment of InvalidGrantError, InvalidRequestError and the
fall-through AbortError is reverted: session.ts only checks the class,
so those messages carried the detail into default output for nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@wes-shaw
wes-shaw force-pushed the automation-token-identity-error branch 2 times, most recently from e0a9fd1 to ba7c1b7 Compare August 3, 2026 20:39
Comment thread packages/cli-kit/src/private/node/session/exchange.ts Outdated
@wes-shaw
wes-shaw requested a review from isaacroldan August 4, 2026 18:20
@wes-shaw
wes-shaw added this pull request to the merge queue Aug 5, 2026
Merged via the queue into Shopify:main with commit 916ae6f Aug 5, 2026
28 checks passed
@wes-shaw
wes-shaw deleted the automation-token-identity-error branch August 5, 2026 16:48
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.

2 participants