Skip to content

docs: add #807 throw-site inventory (stage 1) - #1121

Open
SHAI-krishna-Dobhal wants to merge 2 commits into
google:mainfrom
SHAI-krishna-Dobhal:docs/807-throw-inventory
Open

SHAI-krishna-Dobhal wants to merge 2 commits into
google:mainfrom
SHAI-krishna-Dobhal:docs/807-throw-inventory

Conversation

@SHAI-krishna-Dobhal

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:
#583 added typed errors, but every one of the 340 throw new Error( sites in core/src still throws a bare Error. Callers can only tell failures apart by matching on error.message.

Solution:
Adds issue-807/throw-inventory.md, a Stage 1 inventory sorting every site into: fits an existing type (185), needs one of 6 proposed new types (105), or keep as bare Error (49). It also confirms StaleSessionError isn't needed, lists ~15 rewraps that drop the original error, and proposes a per-area order for Stage 2.

Testing Plan

Documentation only; no code changes. Site count is reproducible with:
git grep -n "throw new Error(" -- 'core/src/**/*.ts' | wc -l → 340 on 934d6bf.

Unit Tests:

  • I have added or updated unit tests for my change. (N/A, docs only)
  • All unit tests pass locally. (N/A, docs only)

Manual End-to-End (E2E) Tests:

N/A, docs only.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas. (N/A)
  • I have added tests that prove my fix is effective or that my feature works. (N/A)
  • New and existing unit tests pass locally with my changes. (N/A)
  • I have manually tested my changes end-to-end. (N/A)
  • Any dependent changes have been merged and published in downstream modules.

@google-cla

google-cla Bot commented Oct 22, 2026

Copy link
Copy Markdown

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.

@Varun-S10 Varun-S10 self-assigned this Oct 23, 2026
@Varun-S10

Copy link
Copy Markdown
Contributor

Hi @SHAI-KrishnaDobhal, Thank you for raising the PR and putting together the Stage 1 inventory. Could you please move the file under docs/ (instead of root issue-807/), rebase onto latest main, and run npx prettier --write on it?
Meanwhile, we are forwarding your proposed error types and Stage 2 questions to the team for review.
Hi @kalenkevich, Could you please take a look?

@Varun-S10 Varun-S10 added the request clarification [Status] The maintainer need clarification or more information from the author label Oct 23, 2026
Classify all 340 bare `throw new Error(` sites in core/src (@934d6bf)
by failure mode, as requested on google#583:

- 185 map to an existing typed error (154 InputValidationError,
  15 NotFoundError, 7 ToolExecutionError, 6 AlreadyExistsError,
  3 SessionNotFoundError)
- 105 need a new type; 6 proposed: UnsupportedFeatureError,
  UpstreamServiceError, InvalidWorkflowGraphError, SkillFormatError,
  AuthenticationError, ServiceNotConfiguredError
- 49 stay bare Error (invariants, abstract stubs, unreachable)
- 1 is a JSDoc example, not code

Also confirms StaleSessionError is not needed (adk-js reloads stale
sessions instead of throwing), lists ~15 rewraps that drop the
original error, and proposes a per-area order for stage 2.

Refs google#807
@SHAI-krishna-Dobhal

Copy link
Copy Markdown
Contributor Author

@Varun-S10! Done:

  • Moved the inventory to docs/issue-807-throw-inventory.md
  • Rebased onto latest main (bb9a49a)
  • Ran npx prettier --write on it (--check passes)

@Varun-S10 Varun-S10 added needs review [Status] The PR/issue is awaiting review from the maintainer and removed request clarification [Status] The maintainer need clarification or more information from the author labels Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review [Status] The PR/issue is awaiting review from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants