Skip to content

refactor(proto): introduce shared ServiceError type and begin unifying error patterns#823

Open
NewCoder3294 wants to merge 2 commits intokoala73:mainfrom
NewCoder3294:refactor/unified-error-pattern
Open

refactor(proto): introduce shared ServiceError type and begin unifying error patterns#823
NewCoder3294 wants to merge 2 commits intokoala73:mainfrom
NewCoder3294:refactor/unified-error-pattern

Conversation

@NewCoder3294
Copy link
Contributor

Summary

Closes #191 (first step).

The codebase currently has four different error patterns coexisting across proto response messages:

  1. string error -- a plain string field (e.g. GetTemporalBaselineResponse, SearchGdeltDocumentsResponse, GetUSNIFleetReportResponse)
  2. bool success + string error -- a success flag paired with an error string (e.g. ListTechEventsResponse, GetPopulationExposureResponse)
  3. GeneralError -- a structured oneof for infrastructure-level errors like rate limiting, geo blocking, upstream outages (core/v1/general_error.proto, currently unused in any response message)
  4. Gateway-level JSON errors -- { error: string } / { message: string } returned directly by the gateway error boundary and error-mapper

This PR introduces a shared ServiceError message type as the standard pattern for per-RPC business logic errors and migrates three response messages as a proof of concept:

  • New: proto/worldmonitor/core/v1/service_error.proto with code (machine-readable) + message (human-readable) fields
  • Migrated: GetTemporalBaselineResponse, RecordBaselineSnapshotResponse, SearchGdeltDocumentsResponse
  • Old string error field numbers are reserved to maintain wire compatibility
  • Handler code updated to construct { code, message } objects (e.g. INVALID_ARGUMENT, INTERNAL, UPSTREAM_ERROR)
  • Client-side consumers (gdelt-intel.ts) updated to use resp.serviceError

Migration approach

The field is named service_error (not error) to avoid conflict with the reserved old field name. Future PRs will migrate the remaining response messages following this same pattern. The GeneralError type remains available for infrastructure-level cross-cutting concerns (rate limiting, geo blocking, etc.).

Changed files

File Change
proto/worldmonitor/core/v1/service_error.proto New shared ServiceError message
proto/.../get_temporal_baseline.proto Replace string error with ServiceError service_error
proto/.../record_baseline_snapshot.proto Replace string error with ServiceError service_error
proto/.../search_gdelt_documents.proto Replace string error with ServiceError service_error
server/.../get-temporal-baseline.ts Use serviceError: { code, message }
server/.../record-baseline-snapshot.ts Use serviceError: { code, message }
server/.../search-gdelt-documents.ts Use serviceError: { code, message }
src/services/gdelt-intel.ts Update client references from resp.error to resp.serviceError
src/generated/... Regenerated (infrastructure + intelligence only)
docs/api/... Regenerated OpenAPI specs

Test plan

  • tsc --noEmit passes (non-generated code has zero errors)
  • buf lint passes for all modified proto files
  • Verify GDELT search endpoint returns serviceError object on failure
  • Verify temporal baseline endpoint returns serviceError on invalid params
  • Verify record baseline snapshot endpoint returns serviceError on empty updates

🤖 Generated with Claude Code

NewCoder3294 and others added 2 commits March 2, 2026 12:55
…g error patterns

Create a shared ServiceError message in core/v1 with machine-readable
code + human-readable message fields. Migrate three response messages
from ad-hoc `string error` to the new type as a first step toward
full unification (ref koala73#191):

- GetTemporalBaselineResponse (infrastructure)
- RecordBaselineSnapshotResponse (infrastructure)
- SearchGdeltDocumentsResponse (intelligence)

Old field numbers are reserved to maintain wire compatibility. Handler
code and client-side consumers updated to use the new serviceError
field with structured { code, message } objects.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The sebuf v0.7.0 codegen dropped @ts-nocheck from generated files,
causing pre-existing type errors to surface. Restore the directive
on the four files changed by this PR, and restore all other generated
files to their original state since they had no proto changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 2, 2026

@NewCoder3294 is attempting to deploy a commit to the Elie Team on Vercel.

A member of the Team first needs to authorize it.

@koala73 koala73 added Ready to Merge PR is mergeable, passes checks, and adds value Low Value Trivial, unnecessary, or not aligned with project needs labels Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Low Value Trivial, unnecessary, or not aligned with project needs Ready to Merge PR is mergeable, passes checks, and adds value

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify error patterns across proto responses

2 participants