google / google/adk-js

Audit bare `throw new Error` sites and adopt the typed-errors module (232 throws across 90 files)

Open
#807 0 comments 0 reactions 1 assignee Claimed by @Varun-S10 View on GitHub
enhancement
Dominant language
TypeScript
Stars
1.4k
Forks
205
Avg merge
3d 16h
Merged PRs (30d)
92

Description

Follow-up to **#583** (port the typed-errors module). Gated on that PR merging.

This issue captures the reviewer request left on #583:

> **@kalenkevich, 2026-07-31:** 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?

and the scope boundary the PR author drew:

> 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.

### The problem

#583 adds five typed error classes but changes **no** existing throw site, by design. So the module ships unused, and the original motivation stands untouched: callers still cannot branch on failure mode without string-matching `error.message`.

Measured on `main` (`bb2dd8f`):

```
$ git grep -c "throw new Error(" -- 'core/src/**/*.ts'
232 total across 90 files
```

Worst offenders:

| File | Bare throws |
|---|---:|
| `core/src/skills/loader.ts` | 11 |
| `core/src/workflow/utils/graph_validation.ts` | 9 |
| `core/src/runner/runner.ts` | 8 |
| `core/src/artifacts/file_artifact_service.ts` | 8 |
| `core/src/agents/llm_agent.ts` | 8 |
| `core/src/integrations/agent_registry/agent_registry.ts` | 7 |
| `core/src/code_executors/agent_engine_sandbox_code_executor.ts` | 6 |
| `core/src/agents/context.ts` | 6 |

That count is `core/src` only and excludes sites that return `undefined` instead of throwing — which the #583 description calls out as the same problem wearing a different hat.

### Deliverable, in two stages

**Stage 1 — the inventory (what @kalenkevich asked for).** Classify every throw site by failure mode:

- maps to an existing ported type (`NotFoundError`, `AlreadyExistsError`, `SessionNotFoundError`, `InputValidationError`, `ToolExecutionError`)
- needs a **new** type — and if so, propose it, with a note on whether adk-python already has one
- should stay a bare `Error` (genuine invariant violations / programmer error)

Also worth resolving in this pass: adk-python has **`_stale_session_error.py`**, which #583 did not port. The leading underscore marks it private upstream, so skipping was defensible — but the audit should confirm nothing in adk-js needs it.

**Stage 2 — adoption**, driven by the inventory, and **split by area rather than landed as one sweep**. A 232-site refactor as a single PR is unreviewable and will conflict with most of the ~43 open PRs.

### Sequencing constraint

The PR author flagged that `core/src/sessions/**` collides with in-flight session and evaluation work. Check the open-PR surface before touching an area — `core/src/sessions/`, `core/src/agents/` and `core/src/artifacts/` all currently have PRs against them. Prefer starting with an area nothing is touching (`core/src/skills/loader.ts` and `core/src/workflow/utils/graph_validation.ts` look clear, and are the two densest files).

### Note on scope

Adopting a typed error where callers previously caught a bare `Error` is **not** behaviour-preserving for anyone matching on `error.message`, and `instanceof` relationships were deliberately kept flat in #583 (no `AdkError` base, no `SessionNotFoundError extends NotFoundError`). So each adoption PR should state whether it changes what a `catch` block sees, and adk's own semver policy should decide whether that needs a major.

### Related

- **#583** — the port this follows up on
- **#806** — wire `errorType` into telemetry (the other #583 follow-up)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.