Skip to content

feat(ts): ts_error_handling=result — typed Result return for es-mode client#210

Open
hishamank wants to merge 4 commits into
feat/ts-es-transportfrom
feat/ts-es-result-errors
Open

feat(ts): ts_error_handling=result — typed Result return for es-mode client#210
hishamank wants to merge 4 commits into
feat/ts-es-transportfrom
feat/ts-es-result-errors

Conversation

@hishamank

Copy link
Copy Markdown
Collaborator

What

Adds an opt-in ts_error_handling=result option to protoc-gen-ts-client. In protobuf-es mode it switches unary client methods from throwing to returning a typed discriminated union, so RPC error handling is compiler-checked instead of try/catch over unknown.

const r = await client.getAccount({ id });
if (!r.ok) {
  const e = r.error; // ValidationError | ApiError | NotFoundError | LoginError | ...
  if (e instanceof ValidationError) { /* e.violations */ }
  else if (e instanceof ApiError) { /* e.statusCode, e.body */ }
  else switch (e.$typeName) { case "pkg.NotFoundError": /* e.resourceType */ }
  return;
}
r.data; // Account (typed, non-null)

Default (throwing) behavior and hand-rolled output are unchanged. The flag fails loud without ts_runtime=protobuf-es.

Why

Per a product directive: when consumers use the protobuf-es client, errors should be typed and returned, not thrown — a method should return either data or a typed error.

Design

  • Result shape{ ok: true; data: T; error?: undefined } | { ok: false; data?: undefined; error: E }. ok discriminant and always-present data/error (inactive one undefined), so both if (r.ok) and const { data, error } work.
  • Typed error union — sebuf has no per-RPC error declaration (any handler may return any *Error), so ClientError = ValidationError | ApiError | <every proto *Error type>. A shared result.ts carries a structural registry + decodeError (400+violationsValidationError; else first *Error whose JSON marker keys all match → typed proto error, narrowable via protobuf-es $typeName; fallback ApiError). This mirrors the Python client's _ERROR_CLASSES.
  • Scope — unary only. SSE keeps AsyncGenerator<T> and still throws on stream-open (a single Result doesn't fit a stream). handleError is emitted only when an SSE method still needs it.

Tests / verification

  • es-result golden fixtures: with proto errors (result_errors.proto), without (get_params), and mixed unary+SSE (sse) — each in its own golden subdir since result.ts is invocation-global.
  • tsc --noEmit typecheck of the es-result goldens against the real @bufbuild/protobuf@2.12.1 — proves the Result return, the ClientError union, the registry, and $typeName narrowing all compile.
  • Unit tests for ParseErrorHandling / ValidateRuntimeOptions (result-without-es fails loud).
  • Full go test ./internal/... green; existing es/hand-rolled goldens byte-identical (no behavior change in throw mode).
  • Docs: new "Typed Result returns" section in docs/client-generation.md.

Base

Targets feat/ts-es-transport (#207) since it reuses that branch's es transport + modules machinery. Rebase onto main once #207 (and its base) land.

🤖 Generated with Claude Code

…client

Adds an opt-in ts_error_handling=result plugin option to protoc-gen-ts-client.
In protobuf-es mode it switches unary methods from throwing to returning a typed
discriminated union, so RPC error handling is compiler-checked instead of
try/catch over `unknown`. Default (throw) behavior and hand-rolled output are
unchanged; the flag fails loud without ts_runtime=protobuf-es.

- Flag plumbing: ErrorHandling + ParseErrorHandling + ValidateRuntimeOptions in
  tscommon/runtime.go; EmitContext.ErrorHandling; cmd/protoc-gen-ts-client
  accepts/validates the option and threads it into New.

- Shared result.ts (tscommon/result.go), emitted client-side only in es+result:
  * Result<T,E> = { ok: true; data: T; error?: undefined }
                | { ok: false; data?: undefined; error: E }
    (ok discriminant + always-present data/error, so `if (r.ok)` and
    `const { data, error }` both work).
  * ClientError = ValidationError | ApiError | <every proto *Error type>.
  * decodeError: 400+violations -> ValidationError; else a structural registry
    (first *Error whose JSON marker keys are all present) -> typed proto error,
    narrowable via protobuf-es $typeName; fallback ApiError. Mirrors the Python
    client's _ERROR_CLASSES. Registry/fromJson omitted when there are no proto
    *Error messages.

- Unary methods return Promise<Result<Out, ClientError>> and never throw. SSE
  methods keep AsyncGenerator<T> and still throw on stream-open; handleError is
  emitted only when an SSE method references it (else noUnusedLocals trips).

- Extend the @bufbuild/protobuf import machinery to carry the DescMessage
  type-only symbol (used by the registry).

- Tests: es-result golden fixtures with proto errors (result_errors.proto),
  without (get_params), and mixed unary+SSE (sse) — each in its own golden
  subdir since result.ts is invocation-global — plus a tsc --noEmit typecheck
  against real @bufbuild/protobuf, and unit tests for ParseErrorHandling /
  ValidateRuntimeOptions (result-without-es fails loud).

- Docs: new "Typed Result returns" section in docs/client-generation.md.

Branched off #207 (feat/ts-es-transport); reuses its es transport + modules
machinery.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase #210's ts_error_handling=result work onto the freshly-rebased
#207. Combines #210's Result-mode surfaces (Result<T,ClientError>
return path, result.ts shared module, decodeError, ValidateRuntimeOptions,
es-result goldens) with everything #207 pulled from main (#199 oneof
fixes, record-map `{ [key: string]: T }`, reserved-name aliasing/harness).

- Resolve inprocess_golden_test.go reserved-name conflict: adopt #207's
  new aliasing semantics (reserved names aliased, not fatal) while keeping
  #210's 3-arg New(plugin, runtime, errorHandling) signature.
- Regenerate hand-rolled + es + es-result TS goldens.
- gitignore the es-result node_modules typecheck symlink (parity with es).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hishamank added a commit that referenced this pull request Jul 22, 2026
…nctions

Brings the rebased #210 (es transport #207 + Result mode + main fixes)
underneath #211's standalone tree-shakeable es-mode client.

Combined result:
- #211 standalone functions preserved: each RPC is an `export async
  function` (unary) / `export async function*` (SSE) taking a required
  per-call `RequestOptions`, the shared client.ts base RequestOptions +
  RefClient/EmitClientModule, `{Service}RequestOptions extends
  RequestOptions` for typed-header services, top-level handleError, and
  `const path` when there are no path params.
- #210 Result mode preserved underneath: standalone unary functions in
  result mode return `Promise<Result<T, ClientError>>` via decodeError;
  #207 es transport, oneof fixes, and reserved-name handling remain.

Regenerated es, es-result, and hand-rolled client goldens plus ts-server
goldens. Added the missing sebuf/http/headers_pb.ts transitive es fixture
(analogue of the committed annotations_pb.ts) that es_headers_pb.ts imports,
so the es golden typecheck resolves.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@SebastienMelki SebastienMelki left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks Hicham — the Result mode is well-designed and composes cleanly with the es transport. The three-tier decode (ValidationError → structural registry → ApiError) mirrors the Python client nicely, and the conditional handleError omission for SSE is handled correctly. Ready to merge once #207 lands and the one suggestion below is addressed.


Suggestion (non-blocking but worth fixing before consumers hit it)

Empty-field error messages match everything in the registry

In result.go, the registry entry for a proto *Error message uses its field JSON names as discriminator keys:

if (keys.every((k) => k in json)) {

[].every(...) returns true for any predicate. So a message InternalError {} (zero fields) would match every response body, shadowing all subsequent registry entries. This is a realistic edge case — simple sentinel error messages with no payload are common.

Fix options:

  • Skip zero-field error messages from the registry entirely (they can't be structurally distinguished anyway — fall through to ApiError)
  • Or require keys.length > 0 && keys.every(...) in the emitted decodeError

Either way, consider documenting that the registry relies on distinct field sets for disambiguation — like the Python client does.


Observations (all positive)

  • Result type shape is correct{ ok: true; data: T; error?: undefined } | { ok: false; data?: undefined; error: E } enables both if (r.ok) narrowing and const { data, error } destructuring.
  • ValidateRuntimeOptions correctly rejects result without protobuf-es — good fail-loud behavior.
  • handleError omission is precisely scoped: emitted only when SSE methods exist in result mode, avoiding noUnusedLocals violations.
  • Test coverage is solid: three golden subdirs (no-errors, typed-errors, mixed SSE), plus tsc --noEmit typecheck against real @bufbuild/protobuf, plus unit tests for parse/validation.
  • DescMessage import correctly added as type-only to the @bufbuild/protobuf import machinery.
  • No regressions — existing es and hand-rolled goldens byte-identical.

Nice work on this one — the typed Result pattern will be a big DX improvement for consumers.

hishamank and others added 2 commits July 23, 2026 17:58
A proto *Error with no fields (e.g. `message InternalError {}`) registers in
ERROR_REGISTRY with an empty key set. The match test `keys.every((k) => k in
json)` returns true for `[]`, so a zero-field error matched EVERY response body
and shadowed every registry entry after it.

Emit `keys.length > 0 && keys.every(...)` in the generated decodeError loop so a
zero-field *Error is skipped at match time and never shadows a real error. Add a
comment in the emitted result.ts and a note in docs/client-generation.md that
disambiguation relies on distinct, non-empty field sets (mirrors the Python
client). Add a zero-field InternalError to the es-result typed-errors fixture to
lock in the non-shadowing behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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