Skip to content

Feat: Port the typed-errors module (errors/) from adk-python - #583

Open
AmaadMartin wants to merge 2 commits into
google:mainfrom
AmaadMartin:feat/core-errors-module-parity
Open

Feat: Port the typed-errors module (errors/) from adk-python#583
AmaadMartin wants to merge 2 commits into
google:mainfrom
AmaadMartin:feat/core-errors-module-parity

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

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

No existing issue.

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

Problem:
adk-js has no typed errors. Services signal "the thing you asked for does not exist", "the thing you are creating already exists" and "something genuinely broke" all with a bare Error (or by returning undefined), so the only way for a caller to branch on the failure mode is to string-match error.message — which breaks the moment a message is reworded. adk-python solved this with a small errors/ module; adk-js has no equivalent, so each in-flight port (session services, the evaluation framework) would otherwise invent its own incompatible error classes.

Solution:
Port adk-python/src/google/adk/errors/ to core/src/errors/, one TypeScript file per Python file, and export the six symbols from core/src/common.ts. The change is purely additive: 5 new source files, 5 new test files, and 8 added lines in common.ts. No existing throw site is touched, no dependency is added, and git diff main --stat is 387 insertions / 0 deletions.

New file (core/src/errors/) Exported symbols Parity source
not_found_error.ts NotFoundError not_found_error.py
already_exists_error.ts AlreadyExistsError already_exists_error.py
session_not_found_error.ts SessionNotFoundError session_not_found_error.py
input_validation_error.ts InputValidationError input_validation_error.py
tool_execution_error.ts ToolErrorType, ToolExecutionError tool_execution_error.py
import {NotFoundError} from '@google/adk';

try {
  await evalSetManager.getEvalSet(appName, evalSetId);
} catch (e: unknown) {
  if (e instanceof NotFoundError) return undefined; // benign miss
  throw e; // real failure, propagate
}

Parity decisions, and which rule won each one. Every default message, member name and member value was diffed field-by-field against the Python source; local TS convention was only allowed to win where nothing crosses a process boundary.

  1. Flat hierarchy — parity wins. In Python NotFoundError/AlreadyExistsError extend Exception while SessionNotFoundError/InputValidationError extend ValueError ("for backward compatibility" with callers already catching ValueError). TypeScript has no ValueError analogue, so all five extend Error directly. Deliberately not done: making SessionNotFoundError extend NotFoundError, or introducing an AdkError base. Either would invent a catch relationship that does not exist upstream and would silently change which catch block wins. A test in each file pins this by asserting an instance is not an instance of a sibling.
  2. error_typeerrorType — local convention wins. The property name never leaves the process. The enum values are unchanged, because those do cross the boundary: they populate the OpenTelemetry error.type span attribute. All nine member names/values are byte-identical to tool_execution_error.py, and a test asserts the exact list, its order, and a length of exactly 9 (so an added member also fails).
  3. No enum→string normalisation. Python needs error_type.value if isinstance(error_type, ToolErrorType) else error_type because an enum member is not its value. A TypeScript string-enum member is its string value at runtime, so direct assignment is already equivalent and the branch would be dead code. Pinned by a test asserting new ToolExecutionError('x', ToolErrorType.NOT_FOUND).errorType === 'NOT_FOUND'.
  4. No separate message field. Python sets self.message because Exception does not expose one; JS Error already has .message from super(message). A redundant field would shadow the built-in.
  5. No Object.setPrototypeOf(this, X.prototype). That shim is only needed when class is downlevelled to an ES5 constructor function, and this repo never does that: tsconfig.json sets "target": "ES2020", and core/build.js:9-12 sets esbuild targets node10.4 for the Node build and chrome58/firefox57/safari11 for the browser build — all emit native classes. Verified on the actual build output: core/dist/web/errors/not_found_error.js (the most aggressive target) contains a literal class NotFoundError extends Error, and importing it under Node gives instanceof NotFoundError: true | instanceof Error: true | not sibling: true | name: NotFoundError. There are also zero occurrences of setPrototypeOf anywhere in core/src, dev/src or integrations/src, so adding it to five classes would be unexplained ceremony.
  6. errorType is written but not yet read inside this repo — intentional. core/src/telemetry/tracing.ts has no error.type span-attribute handling today; porting adk-python's resolve_error_type is separate follow-up work. This is a public API property being ported for parity, not dead internal config, so please don't read it as an unwired knob. Refactoring existing bare-Error throws (e.g. core/src/sessions/database_session_service.ts) is likewise deliberately out of scope — it would collide with the in-flight session/evaluation work.

Barrel placement. core/src has no per-directory index.ts; core/src/common.ts is the real barrel and is re-exported wholesale by both index.ts (Node) and index_web.ts (browser). These classes are platform-neutral, so they go in common.ts only — adding them to index.ts as well would be redundant. Exported as values (export {…}, not export type {…}), since they are runtime classes plus a runtime enum.

Collision check. No path containing errors is touched by any other open PR. The only shared file is core/src/common.ts, where a handful of open PRs add unrelated export lines; this PR is branched from main rather than stacked on any of them, so expect at most a trivial add/add conflict in common.ts depending on merge order.

Testing Plan

Please describe the tests that you ran to verify your changes.

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

5 new files under core/test/errors/, 34 tests, all passing:

$ npx vitest run --project unit:core core/test/errors
 ✓ |unit:core| core/test/errors/session_not_found_error_test.ts (6 tests)
 ✓ |unit:core| core/test/errors/not_found_error_test.ts (6 tests)
 ✓ |unit:core| core/test/errors/already_exists_error_test.ts (6 tests)
 ✓ |unit:core| core/test/errors/input_validation_error_test.ts (6 tests)
 ✓ |unit:core| core/test/errors/tool_execution_error_test.ts (10 tests)
 Test Files  5 passed (5)
      Tests  34 passed (34)

Every test imports from @google/adk rather than a relative path, so a missing or misspelt barrel export fails the suite at import time.

Coverage: 100% statements / branches / functions / lines on core/src/errors/, measured with:

npx vitest run --project unit:core core/test/errors --coverage --coverage.include='core/src/errors/**'

No coverage-ignore pragmas, and no structure was compromised to reach the number.

Coverage is not proof, so each test file was mutation-tested — the source was broken and the suite confirmed red, then reverted:

# Mutation Result
1 not_found_error.ts: default message …not found.…not found! defaults the message when none is suppliedexpected 'The requested item was not found!' to be 'The requested item was not found.'
2 already_exists_error.ts: delete the this.name = 'AlreadyExistsError'; line sets nameexpected 'Error' to be 'AlreadyExistsError'
3 session_not_found_error.ts: extends Errorextends NotFoundError (invent a hierarchy) is not an instance of a sibling error classexpected SessionNotFoundError: Session not found. to not be an instance of NotFoundError
4 input_validation_error.ts: drop the default parameter (message = 'Invalid input.'message: string) defaults the message when none is suppliedexpected '' to be 'Invalid input.'
5 tool_execution_error.ts: remove the GATEWAY_TIMEOUT member matches the adk-python members, in declaration orderexpected [ … …(6) ] to deeply equal [ … …(7) ]
6 tool_execution_error.ts: drop the readonly parameter property so errorType is never stored ✗ 3 tests — expected undefined to be 'BAD_REQUEST', … 'NOT_FOUND', … '500'
7 tool_execution_error.ts: BAD_GATEWAY = 'BAD_GATEWAY''BadGateway' ✗ 2 tests — expected 'BadGateway' to be 'BAD_GATEWAY' and the member-table assertion
8 common.ts: delete the NotFoundError barrel export line ✗ whole suite at import — TypeError: NotFoundError is not a constructor

Edge cases covered explicitly: new X('') yields '' and not the default (only undefined triggers a TS default parameter), new X(undefined) yields the default, and a message containing $ replacement metacharacters ("a $& b $' c") is stored verbatim with no sanitisation.

No integration tests were added, and that is deliberate: nothing in the repo throws or catches these types yet, so an "integration" test could only re-assert the unit behaviour through more indirection. The real integration surface arrives with the follow-up ports.

No new suppressions anywhere in the diff — no any, as any, @ts-expect-error, @ts-ignore, eslint-disable, or coverage-ignore pragma, in source or in tests (verified by grepping git diff main -U0 for all of them: zero hits).

Manual End-to-End (E2E) Tests:

From the repo root, run the checks CI runs:

npm install && npm run build
npx vitest run --project unit:core core/test/errors   # 34 passed
npm run lint                                          # clean
npm run format:check                                  # clean
npm run docs:check                                    # TypeDoc, warnings-as-errors: clean
bash scripts/check_license.sh                         # all headers valid

Then confirm the symbols really are public, against the built package rather than the source alias:

node --input-type=module -e "
import {ToolErrorType, ToolExecutionError} from '@google/adk';
const e = new ToolExecutionError('boom', ToolErrorType.GATEWAY_TIMEOUT);
console.log(e.name, e.message, e.errorType, e instanceof Error);
"
# ToolExecutionError boom GATEWAY_TIMEOUT true

Constructing each of the four no-arg errors from the built package prints its parity default: The requested item was not found., The resource already exists., Session not found., Invalid input.

One honest note on npm run ts:check: it is not part of validation.yaml and is currently red (366 errors across 67 files). Every one of those 67 files is a file this PR does not touch, and there are zero errors in core/src/errors/ or core/test/errors/. This PR neither adds to nor fixes that backlog.

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.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Amaad Martin added 2 commits July 30, 2026 11:08
Port the five error types from adk-python's errors/ package so TypeScript
callers can branch on a failure mode with instanceof instead of matching
error.message strings.

All five extend Error directly. adk-python splits them across Exception and
ValueError, but TypeScript has no ValueError analogue, so mirroring the split
would invent a catch relationship that does not exist upstream.

ToolErrorType keeps the exact adk-python member names and values because they
are written into the OpenTelemetry error.type span attribute.
Pin the parity-critical surface: the default message of each error, the
explicit .name, the flat hierarchy (each error is NOT an instance of a
sibling), and the exact ToolErrorType member set, order and count.

Every test imports from @google/adk so a missing barrel export fails the
suite at import time.
@kalenkevich

Copy link
Copy Markdown
Collaborator

Can you go though the whole app and create an essential list of all the places where we throw errors and what other error types we need to create?

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