fix(deploy): surface original error types in preflight steps. #1525
Conversation
…ors to telemetry classification
|
Claude Security Review: the review did not actually analyze this PR (model took 0 turns — the skill likely failed during setup). See the run for details; a later push or re-run is needed for a real review. |
Package TarballHow to installgh release download pr-1525-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.0.tgz |
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for tackling this — strongly typed errors and richer telemetry classification are great improvements. I have a few concerns, primarily around inconsistencies that I think will undermine the PR's stated goal of reducing the unknown error rate.
The biggest one: in setupApiKeyCredentialProvider you correctly wrap AWS-credential failures in AwsCredentialsError, but in setupSingleOAuth2Provider and setupPaymentCredentialProviders the equivalent paths just mutate error.message on the original SDK error. The error's name (and therefore its telemetry classification) is unchanged, so the OAuth/payment paths will still fall through to UnknownError in classifyError — defeating the whole point of the change for those paths. Details inline.
A couple of smaller items also inline.
|
Addressed the main concern by rewrapping the error instead of mutating. most of these are symptoms of the existing implementation. Ignored comment about adding unit tests for a map. |
|
Claude Security Review: the review did not actually analyze this PR (model took 0 turns — the skill likely failed during setup). See the run for details; a later push or re-run is needed for a real review. |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Two issues that need a quick fix before merge. Most of the substantive feedback was already raised by the previous bot review and addressed; flagging these two new ones.
081e2dc to
1e6fdda
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
Problem
The deploy command has a very high error rate, with the majority coming from an
unknownsource. This makes it difficult to know if customers are blocked on a legitimate bug, or are simply using the command incorrectly.Solution
Error.Notes
Testing