Skip to content

Populate error state in UI when agent communication fails#1150

Open
zeroasterisk wants to merge 1 commit intomainfrom
feat/error-feedback
Open

Populate error state in UI when agent communication fails#1150
zeroasterisk wants to merge 1 commit intomainfrom
feat/error-feedback

Conversation

@zeroasterisk
Copy link
Copy Markdown
Collaborator

This PR improves error handling in the shell client by populating the existing #error state when agent communication fails, making errors visible in the main UI.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the A2UILayoutEditor to manage an internal error state, ensuring it is reset before a request and populated upon failure. A review comment suggests using 'instanceof Error' for safer error handling to avoid potential runtime crashes when non-Error objects are caught.

return response;
} catch (err) {
this.snackbar(err as string, SnackType.ERROR);
this.#error = (err as Error).message || String(err);
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.

medium

The type assertion err as Error is potentially unsafe because in JavaScript/TypeScript, any value can be thrown (including strings, null, or undefined). If err is null or undefined, accessing .message will cause a runtime TypeError, crashing the error handler itself. Using instanceof Error is a safer and more idiomatic way to handle unknown errors.

Suggested change
this.#error = (err as Error).message || String(err);
this.#error = err instanceof Error ? err.message : String(err);

Copy link
Copy Markdown
Collaborator

@ditman ditman left a comment

Choose a reason for hiding this comment

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

I actually removed the whole snackbar shebang when migrating the shell app to lit v0.9, so this won't land cleanly. If we want this, we need to restore the snackbar into the (new) shell app. Sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants