fix(samples): retry shell agent-card discovery during startup#830
fix(samples): retry shell agent-card discovery during startup#830stablegenius49 wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a robust retry mechanism with exponential backoff for agent card discovery, which is a great improvement for handling startup race conditions. The new logic is well-encapsulated in a2a-client-factory.ts, thoroughly tested, and cleanly integrated into both the Lit shell client and the dev middleware. The improved loading message is also a nice touch for the user experience. I have one suggestion to enhance the error reporting for better debuggability.
| throw lastError instanceof Error | ||
| ? lastError | ||
| : new Error(`Failed to connect to agent card: ${cardUrl}`); |
There was a problem hiding this comment.
When the retry loop exhausts all attempts, this logic correctly throws an error. However, if lastError is not an instance of Error (e.g., it's a plain object or a string from a library), the original error details are lost in the new generic Error message. This can make debugging more difficult.
To provide more context, you could serialize the lastError into the new error message. A simple if/else block would also improve readability over the ternary.
if (lastError instanceof Error) {
throw lastError;
}
throw new Error(
`Failed to connect to agent card: ${cardUrl}. Last error: ${JSON.stringify(
lastError,
)}`,
);
Summary
Closes #587
How did you test this change?
npm run buildinsamples/client/lit/shellnode --test dist/tests/client-factory.test.jsinsamples/client/lit/shell