fix(electron): use the shared error-category table instead of a second one

Merged
AY
2mo ago
+6-28

Description

What is wrong

bindings/proto-ts/src/convenience/errors_category.ts opens by naming its own scope:

Canonical ErrorCode / rac_result_t → ErrorCategory range table for every TypeScript SDK (Web, React Native, Electron).
[...] Do not invent a second table in an SDK.

bindings/electron/src/errors.ts:70 invents a second table. Web and React Native both import { categoryForCode } from '@runanywhere/proto-ts/convenience/errors_category'; Electron declares its own, described as a port of the commons range table.

The same file already argues against this one level up, about the enums:

ErrorCode, ErrorCategory, and ErrorSeverity are the GENERATED proto enums from idl/errors.proto, re-exported rather than re-declared [...] A local subset would silently collapse the 115 commons codes it did not list, which is exactly the bug this file used to have.

The category table then does the collapsing instead.

The divergence

Evaluating both tables over codes 0..1100, they disagree on 560 of them:

codesshared tableElectron
330-349AUTHINTERNAL
350-369IO (archive/extraction)INTERNAL
370-379VALIDATION (calibration)INTERNAL
400-499COMPONENT (module/service)INTERNAL
500-599CONFIGURATION (platform/adapter)INTERNAL
600-699COMPONENT (backend/runtime)INTERNAL
1-99, 390-399, 1000+UNSPECIFIEDINTERNAL

So an Electron consumer branching on e.category sees INTERNAL where the identical code gives AUTH or CONFIGURATION on Web and React Native.

This is not the commons table being wrong, and the shared file says so explicitly:

C++ commons [...] currently only maps |100|–|329| and falls through to INTERNAL — that is a known under-mapping, deliberately deferred [...] Do NOT "fix" this TS table down to match commons.

Electron's copy is that deferred under-mapping, re-introduced in the SDK layer.

It is reachable: the local table is the fallback for when the wire supplies no category, and proto3 leaves an unset enum at 0, which fromSerializedSdkError treats as absent.

What this changes

Import and re-export the shared categoryForCode, delete the local copy. Six insertions, twenty-eight deletions, no new code. SDKException's two call sites are untouched.

Verification

Ran the codegen (generate_ts.sh, generate_ts_convenience.py, generate_defaults_pool.py, generate_streams.sh), built proto-ts, then built and tested the Electron package:

electron build: rc=0, 0 TypeScript errors
npm test:  tests 241  pass 241  fail 0

Both existing assertions in test/unit/errors.test.ts still pass unchanged, including categoryForCode maps ranges like the canonical table. That is not luck: all eight codes it pins (0, 100, 110, 130, 182, 259, 380, 804) fall in bands where the two tables already agreed, which is exactly why the divergence went unnoticed.

The behaviour change, from the built package:

code 340 -> AUTH           (was INTERNAL)
code 355 -> IO             (was INTERNAL)
code 372 -> VALIDATION     (was INTERNAL)
code 450 -> COMPONENT      (was INTERNAL)
code 520 -> CONFIGURATION  (was INTERNAL)
code 640 -> COMPONENT      (was INTERNAL)

Summary by CodeRabbit

  • Refactor
    • Standardized error categorization to use the shared implementation, ensuring consistent fallback behavior across the application.
  • Documentation
    • Clarified which error-categorization behavior serves as the authoritative fallback when no more specific classification is available.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring

Testing

  • Lint passes locally — bindings/electron has no lint script; npm run build (tsc, 0 errors) is the equivalent gate and passed
  • Added/updated tests for changes — none added on purpose. test/unit/errors.test.ts already pins this function and passes unchanged; per AGENTS.md I do not add tests unless asked

Ran locally in bindings/electron, after the four IDL codegen steps and a proto-ts build:

npm run build   ->  rc=0, 0 TypeScript errors
npm test        ->  tests 241   pass 241   fail 0

Platform-Specific Testing

None of the template's platform sections apply: this changes bindings/electron, which the matrix does not cover (Electron is also missing from the Labels list below).

Labels

None of the listed labels matches. This is bindings/electron, and there is no Electron SDK label in the repo today (ios-sdk, kotlin-sdk, web-sdk, flutter-sdk, rn-sdk, core). Closest by blast radius is none of them, since no other binding is touched. Happy to relabel if you have a preference.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed) — not needed; the shared table already documents itself as canonical for Electron

Screenshots

Not applicable, no UI surface.