refactor(client): unify ObjectError across all transports

[Danny-Devs]

Rearchitected from the previous version. Original approach added a transport-specific SuiGrpcRequestError. This version takes the opposite direction — makes the existing ObjectError transport-agnostic so instanceof ObjectError works consistently across JSON-RPC, GraphQL, and gRPC.

Description

ObjectError was coupled to JSON-RPC — it imported ObjectResponseError from the jsonRpc layer and its fromResponse() factory switched on JSON-RPC-specific codes. gRPC didn't use it at all, throwing plain Error instead. GraphQL used it but passed a baked-in message string.

This PR makes ObjectError transport-agnostic and satisfies each of the five principles you laid out in review:

1. Consistent errors across RPCs. ObjectErrorCode is the transport-agnostic union 'notFound' | 'deleted' | 'unknown'. JSON-RPC distinguishes deleted at the wire level, so it maps deleted → 'deleted' and notExists / dynamicFieldNotFound → 'notFound'; displayError / unknown surface as 'unknown'. gRPC can't distinguish deleted from never-existed (its NOT_FOUND is not specific), so it maps google.rpc.Code.NOT_FOUND (5) → 'notFound' and everything else → 'unknown'. GraphQL omits absent objects without saying why, so it also collapses to 'notFound'. The 'deleted' code is only surfaced where the wire genuinely distinguishes it; the other two transports stay honest.
2. Consumers don't need to understand which RPC is in use. error.code and error.objectId mean the same thing on all three transports. No transport-specific codes leak through the unified surface.
3. instanceof works consistently across all three clients. Single ObjectError class in client/errors.ts; gRPC and GraphQL now emit ObjectError instead of plain Error. GraphQLResponseError now extends SuiClientError (previously extended Error), and the GraphQL multi-error path wraps the aggregate in SuiClientError with transportDetails: { $kind: 'graphql' } on the cause — so instanceof SuiClientError is genuinely universal across all three transports, not just for object errors. GetObjectsResponse.objects is narrowed from (Object | Error)[] to (Object | ObjectError)[]; existing instanceof Error checks continue to work unchanged because ObjectError extends Error.
4. Decisions based on error details are not tied to an RPC implementation. ObjectError lives in client/errors.ts with zero dependencies on any transport layer. Consumers act on error.code (three values, universal).
5. RPC-specific detail remains accessible. Adds TransportDetails, a tagged union { $kind: 'jsonRpc' | 'grpc' | 'graphql' } on the SuiClientError base class. Consumers that need the raw JSON-RPC response or the google.rpc.Status narrow on error.transportDetails?.$kind and get typed access to the wire payload.

Implementation notes

• ObjectError.objectId is non-empty by contract. In the rare JSON-RPC case where listOwnedObjects returns an error that identifies no specific object (e.g. a displayError with no object_id), we escalate to the base SuiClientError rather than fabricating a sentinel id. Consumers who catch SuiClientError still catch everything; transportDetails carries the raw wire payload. (Chose this over objectId: string | null or '' sentinels because an empty-string sentinel silently breaks if (error.objectId) checks, and a nullable field forces every consumer to re-handle the case.)
• ObjectError's options arg is optional, and the base-class transportDetails field is honestly optional (no declare readonly transportDetails: TransportDetails narrowing on the ObjectError subclass). Consumers can construct ObjectError directly without ceremony; the SDK still attaches transportDetails at every real construction site.
• AggregateObjectError extends SuiClientError, carrying all errors on .errors. coreClientResolveTransactionPlugin now throws the bare ObjectError when exactly one input is invalid (narrowing preserved) and AggregateObjectError when more than one is invalid — restoring multi-error reporting that was previously flattened to a concatenated-message plain Error. Consumers that catch SuiClientError catch both cases uniformly.
• Two exhaustive mapper helpers in jsonRpc/core.ts (mapJsonRpcObjectErrorCode + extractObjectIdFromResponseError) use the satisfies never house pattern from client/utils.ts — if the upstream wire protocol adds a new ObjectResponseError code, the helpers stop compiling.
• gRPC unexpected-oneof case now throws SuiClientError. Previously grpc/core.ts pushed new Error(...) into the response array when the oneof was neither 'object' nor 'error'. That case represents a programmer error / proto drift, not per-object data, so it now throws SuiClientError — still catchable via catch (e) { if (e instanceof SuiClientError) ... }, but no longer surfaced as data.
• instanceof checks in the composed methods now use ObjectError. The narrowed (Object<Include> | ObjectError)[] slot type lets CoreClient.getObject, CoreClient.getDynamicField, and core-resolver.ts use instanceof ObjectError instead of instanceof Error — the tighter check the narrowed type earns.
• GraphQL preserves the raw input id at the ObjectError construction site. Previously graphql/core.ts pre-normalized the input ids before constructing ObjectError, so a consumer passing '0x9999' saw error.objectId === '0x000...9999' on GraphQL but error.objectId === '0x9999' on JSON-RPC and gRPC. Now GraphQL normalizes only for the server-side lookup and preserves rawId for the error, matching the other two transports.
• SuiClientError, ObjectError, AggregateObjectError, ObjectErrorCode, and TransportDetails are exported from @mysten/sui/client.

Test plan

• test/unit/client/object-error.test.ts — unit tests covering:
  • ObjectError shape: instanceof chain, field access, canonical message generation for all three codes ('notFound' / 'deleted' / 'unknown'), cause preservation, optional-options construction, TransportDetails for all three $kind variants, SimulationError distinguishability.
  • AggregateObjectError: instanceof SuiClientError, exposes .errors, summarizes child ids in the aggregate message.
  • SuiClientError base: transportDetails on the base class, cause preserved independently of transportDetails.
  • JSONRpcCoreClient.listOwnedObjects escalation: displayError / unknown wire codes (no wire-level object id) escalate to bare SuiClientError so ObjectError.objectId stays non-empty; notExists / deleted / dynamicFieldNotFound all produce ObjectError with the correct mapped code and extracted id; plus a universal catch (SuiClientError) contract test pinning compatibility across all five wire codes.
  • JSONRpcCoreClient.getObjects error mapping: 5-row it.each exercising every ObjectResponseError wire code (including deleted → 'deleted'); reference-identity test proving transportDetails.response round-trips the raw wire payload without copy or transformation.
  • GrpcCoreClient.getObjects error mapping: 4-row it.each exercising google.rpc.Code.NOT_FOUND (5) and three non-NOT_FOUND codes (INTERNAL=13, PERMISSION_DENIED=7, OK=0) to pin both branches of the gRPC status-code mapping.
  • GraphQLCoreClient.getObjects error mapping: regression tests for cross-transport objectId parity — ObjectError.objectId preserves the raw input id for both an unnormalized ('0x9999') and a pre-normalized input.
• test/unit/client/core-resolver.test.ts —
  • coreClientResolveTransactionPlugin single-error path: when exactly one tx.object(id) input is unresolved, tx.build() rethrows the original ObjectError instance (reference identity) so instanceof ObjectError / error.code / error.objectId / error.transportDetails work consistently through the transaction resolver path.
  • coreClientResolveTransactionPlugin multi-error path: when more than one input is invalid, tx.build() throws AggregateObjectError with every child ObjectError preserved by reference on .errors.
  • GraphQL universal catch: a schema error from handleGraphQLErrors is catchable via instanceof SuiClientError and carries transportDetails: { $kind: 'graphql' }.
• test/e2e/clients/core/objects.test.ts — two new testWithAllClients cases enforce cross-transport parity: getObjects returns ObjectError with the correct code, objectId, and transportDetails.$kind on all three clients, and getObject throws ObjectError on all three clients.
• pnpm --filter @mysten/sui test — 380 passing, 0 failing (typecheck + unit).
• pnpm --filter @mysten/sui lint — 0 warnings, 0 errors; prettier clean.

---

AI Assistance Notice

• This PR was primarily written by AI.
• I used AI for docs / tests, but manually wrote the source code.
• I used AI to understand the problem space / repository.
• I did not use AI for this PR.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
sui-typescript-docs	Ignored	Preview	Apr 19, 2026 4:28am

[hayes-mysten]

This looks pretty good in theory.

The plan here was for all 3 core client to emit ObjectError instances, but that somehow got missed as the other core clients got built out.

Generally speaking all of the core clients should have identical behavior. I think the correct thing to do here would be to have the grpc and graphql clients use (or extend) the existing ObjectError.

It unfortunate that this currently has a different concept of code that does not match up with what we do in grpc.

I am a little hesitant to merge this as is, because it only complicates the existing inconsistency. I think this is better than what we have on the json rpc side, but I want to try to move towards something consistent rather than fragmenting the clients more

[Danny-Devs]

Searching the repo, $kind is already the convention for polymorphic types (ObjectOwner, TransactionResult, ExecutionError, etc.) per AGENTS.md and .agents/skills/fix-client-bug/SKILL.md. And since ObjectError isn't currently in the public re-exports from client/index.ts (only SuiClientError and SimulationError are), restructuring it is a pure internal refactor with no external backward-compat concerns.

Proposed shape — an abstract ObjectError base class with per-transport subclasses, tagged with $kind for consistency with ObjectOwner/ExecutionError, but with narrowing flowing through instanceof on the subclasses (since errors need to be throw-able and class hierarchies narrow via instanceof rather than via discriminator-on-base):

// client/errors.ts — transport-agnostic abstract base
export abstract class ObjectError extends SuiClientError {
    abstract readonly $kind: 'JsonRpc' | 'Grpc' | 'GraphQL';
}

// client/errors.ts — JSON-RPC variant
export class JsonRpcObjectError extends ObjectError {
    readonly $kind = 'JsonRpc' as const;
    readonly code: string;
    readonly data?: unknown;

    constructor(code: string, message: string, data?: unknown) {
        super(message);
        this.code = code;
        this.data = data;
    }
}

// grpc/errors.ts — gRPC variant (imports Status from proto)
export class GrpcObjectError extends ObjectError {
    readonly $kind = 'Grpc' as const;
    readonly code: number;
    readonly details: readonly { typeUrl: string; value: Uint8Array }[];

    constructor(status: Status) {
        super(status.message);
        this.code = status.code;
        this.details = status.details;
    }
}

// graphql/errors.ts — GraphQL variant
export class GraphQLObjectError extends ObjectError {
    readonly $kind = 'GraphQL' as const;
    readonly code: string;
    readonly extensions?: Record<string, unknown>;

    constructor(code: string, message: string, extensions?: Record<string, unknown>) {
        super(message);
        this.code = code;
        this.extensions = extensions;
    }
}

Consumer narrowing via instanceof:

catch (e) {
    if (e instanceof GrpcObjectError && e.code === GrpcStatusCode.UNAVAILABLE) {
        retryWithBackoff(); // e.details is in scope
    } else if (e instanceof JsonRpcObjectError && e.code === 'notExists') {
        surfaceToUser(e.message);
    } else if (e instanceof ObjectError) {
        surfaceToUser(e.message); // any other variant
    }
}

Why this shape:

• instanceof ObjectError catches any SDK object error regardless of transport — preserves the "one class to catch them all" semantic
• instanceof GrpcObjectError (etc.) gives native TypeScript narrowing to the specific variant — variant-specific fields (code, details, etc.) are in scope after the check
• Each variant's code is typed natively: string for JSON-RPC and GraphQL (matching the current ObjectError.code), number/GrpcStatusCode for gRPC. No shared-field widening — each variant keeps its transport-native code type
• Transport-specific heavyweight data (data, details, extensions) lives on its respective subclass, so autocomplete surfaces only the relevant fields after narrowing
• $kind is preserved as a runtime discriminator (useful for serialization, logging, telemetry) and matches the AGENTS.md convention, even though TS narrowing here flows through instanceof
• Layering: abstract base lives in client/errors.ts (transport-agnostic); each concrete subclass lives in its own transport folder, so client/ stays free of transport-specific imports
• ExecutionError's existing discriminated shape is the direct precedent for the variant pattern — this is the error-class flavor of the same idea

One design question before I draft the PR: should ObjectError become abstract, or stay instantiable as a default JSON-RPC variant? Abstract is stricter (forces every throw site to pick a transport) but requires migrating the existing internal new ObjectError('notExists', ...) throws to new JsonRpcObjectError(...) — mechanical but touches every current throw site. Instantiable is strictly non-breaking but slightly fuzzier about intent. I'm leaning abstract.

Happy to split this into two PRs if it helps review: PR1 = the $kind class hierarchy and migration of existing JSON-RPC throws, PR2 = wiring the gRPC client through GrpcObjectError. That way the type design lands in isolation before any client code moves.

[hayes-mysten]

I don't think this solution solves the original problem well. If I understand the problem correctly, when using the core API you can't tell why an object errored. This solution means you need to know the which RPC implementation is being used, and the specific error code format and meanings for that rpc. The proposed hierarchy is pretty confusing and its not clear and doubles down on the bad error patterns we already have.

I think we probably should take a step back and think through how errors should be surfaced in general.

• core methods (and to some extent top level methods) on all clients should return consistent errors
• consumers of clients should not need to think about or understand which underlying RPC is being used
• we should be able to consistently use instanceof checks across errors from all 3 clients
• if you need to make decisions based on error details, that should not be tied to an rpc implementation
• if an rpc implementation provides additional information, we should expose it

I think given this, what we probably want is:

• A base SuiClientError (exists)
• Specific sub-classes for known error cases (ObjectError exists, but is not used consistently)
• a way to tell what type of object error occured (does not really exist, existing "code" property is inconsistent across implementation)
• a way to get the rpc specific error details (I think this could be attached to the base client via a sub class of a new class like RpcErrorDetails or something (error.rpcDetails.$kind === 'grpc', error.rpcDetails.code = grpc error code?)

I don't have an exact implementation in mind, but I think something that doesn't specialize the error class per implementation would be better here

[clud-bot]

Deep review — this PR does not satisfy the stated requirements

I did a thorough multi-pass review measuring this against Hayes's five requirements from the prior comment. The core goal — instanceof ObjectError parity across transports — is partially achieved, but there are significant design issues, behavioral regressions, and a semver story that isn't honest.

Critical issues

1. TransportDetails is required on ObjectError, leaking transport into the public API (violates req #2)

ObjectError constructor requires transportDetails: TransportDetails — a tagged union with $kind: 'jsonRpc' | 'grpc' | 'graphql'. This means any code constructing, mocking, or testing an ObjectError must manufacture a transport tag. The tests themselves prove the smell: they had to invent const ANY_TRANSPORT: TransportDetails = { $kind: 'graphql' } just to create test errors. If transportDetails is an escape hatch for power users, it should be optional on ObjectError, not mandatory.

2. ObjectErrorCode = 'notFound' | 'unknown' is too lossy (violates req #4)

JSON-RPC's deleted was a distinct, useful code — consumers may need to distinguish "never existed" from "existed but deleted." This PR flattens deleted into 'notFound'. The only way to recover that distinction is now error.transportDetails.$kind === 'jsonRpc' && error.transportDetails.response.code === 'deleted' — which directly violates requirement #4 ("decisions based on error details should not be tied to an RPC implementation"). At minimum, ObjectErrorCode should include 'deleted' as a normalized code. If gRPC/GraphQL can't distinguish it, they emit a subset — that's fine.

3. listOwnedObjects inconsistency — same wire codes get different error types depending on method

For displayError/unknown wire codes:

• getObjects() → returns ObjectError('unknown', batch[idx], ...)
• listOwnedObjects() → throws plain SuiClientError (not ObjectError)

This is because the new ObjectError requires non-null objectId, and displayError/unknown don't carry one in listOwnedObjects context. This is an abstraction design failure, not an unavoidable transport limitation. Code catching ObjectError from listOwnedObjects will silently miss these cases — a behavioral regression from the old ObjectError.fromResponse() path.

4. Resolver drops multi-object error diagnostics

core-resolver.ts previously collected ALL invalid objects and threw a single error with all their messages. Now it throws only the first ObjectError and silently ignores the rest. The comment admits this: "Multi-invalid aggregation is intentionally out of scope." But this makes debugging transactions with multiple missing objects materially worse (fix-one-rerun-repeat). An AggregateObjectError with .errors: ObjectError[] would preserve the new instanceof contract without the regression.

5. declare readonly transportDetails: TransportDetails is a type-level lie

This uses declare to narrow the base class's optional field to required without any runtime enforcement. The base constructor assigns this.transportDetails = options?.transportDetails — if someone passes undefined as any, it's silently undefined at runtime while TS claims it's present. The tests themselves don't trust it (they use err.transportDetails?.$kind with optional chaining). Either enforce it at runtime or stop claiming it's required.

6. Changeset claims minor but ObjectError constructor is source-breaking

The constructor changed from (code: string, message: string) to (code: ObjectErrorCode, objectId: string, options: { transportDetails: TransportDetails }). Anyone constructing ObjectError directly (including in tests/mocks) will break. GetObjectsResponse.objects narrowing from (Object | Error)[] to (Object | ObjectError)[] also changes behavior for code like instanceof Error && !(obj instanceof ObjectError). This should be major, or needs compatibility shims.

Major issues

7. GraphQL overclaims notFound for missing results

GraphQL transport synthesizes ObjectError('notFound', ...) whenever an object isn't in the result page. But absence from a GraphQL response could also mean partial indexing, authorization filtering, backend bugs, or rate limiting — not necessarily "not found." The conservative mapping would be 'unknown' unless the schema explicitly guarantees absence semantics.

8. SuiClientError is NOT actually a universal catch contract

The changeset claims "Use instanceof SuiClientError as the universal catch contract for any error originating from the client." But graphql/core.ts:806-815 throws GraphQLResponseError extends Error (not SuiClientError) and AggregateError for top-level GraphQL failures. The documented catch contract is false.

Summary

The right direction here is clear — all transports should emit ObjectError consistently. But the current implementation has too many regressions and design issues to merge. Minimum fixes needed:

1. Make transportDetails optional on ObjectError
2. Add 'deleted' to ObjectErrorCode
3. Fix listOwnedObjects to emit ObjectError consistently
4. Restore multi-error aggregation in the resolver
5. Runtime-enforce or drop the declare narrowing
6. Mark as major or add compatibility shims

[clud-bot]

critical: declare readonly transportDetails: TransportDetails narrows the base class's optional field to required without runtime enforcement. The base constructor at line 37 just does this.transportDetails = options?.transportDetails — nothing prevents undefined at runtime. Either add a runtime check in the ObjectError constructor (if (!options.transportDetails) throw ...) or stop claiming it's required.

The tests themselves don't trust this guarantee — they use err.transportDetails?.$kind with optional chaining throughout.

[clud-bot]

critical: 'notFound' | 'unknown' is too coarse. The old ObjectError preserved 'deleted' as a distinct code, which consumers use to distinguish "never existed" from "existed but was deleted." Flattening deleted into notFound means the only way to recover that info is to branch on transportDetails.$kind === 'jsonRpc' and inspect the raw payload — exactly the transport coupling Hayes said to avoid.

At minimum add 'deleted' here. If gRPC/GraphQL can't distinguish it, they just never emit that code — the union should be a superset, not a lowest-common-denominator.

[clud-bot]

major: Requiring transportDetails: TransportDetails (not optional) in the public constructor means any code constructing/mocking/testing ObjectError must manufacture a transport-specific tag. This leaks transport concerns into the abstraction layer, violating requirement #2. Make this optional — the transport implementations can always provide it internally.

[clud-bot]

critical: For displayError/unknown wire codes, this throws plain SuiClientError instead of ObjectError. But getObjects() at line 176 maps the same wire codes to ObjectError('unknown', batch[idx], ...) because it has access to the input object id.

This means instanceof ObjectError catches object failures from getObjects but misses them from listOwnedObjects — not unified even within the same transport. The old ObjectError.fromResponse() threw ObjectError for all wire codes.

Root cause: ObjectError now demands a non-null objectId, and displayError/unknown don't carry one here. Fix the abstraction (allow nullable objectId, or use a sentinel) rather than punting to a different error class.

[clud-bot]

major: Previously collected ALL invalid objects and threw a single error listing them all. Now throws only the first one. For transactions referencing multiple missing objects, this turns debugging from "here are all failures" into "fix one, rerun, discover the next" — a real quality regression.

The comment acknowledges this but ships the regression anyway. If preserving instanceof ObjectError matters, add AggregateObjectError extends SuiClientError with .errors: ObjectError[].

[clud-bot]

major: Synthesizing ObjectError('notFound', ...) when an object is absent from the GraphQL result page overclaims certainty. Absence could mean partial indexing, eventual consistency, authorization filtering, resolver bugs, or rate limiting — not just "not found." The GraphQL transport details are empty ({ $kind: 'graphql' }) so there's no recovery path.

Consumers may make real decisions based on code === 'notFound' (create-if-absent, purge cache, etc.) that would be wrong here. Consider 'unknown' unless the GraphQL schema contract explicitly guarantees absence semantics.

[clud-bot]

major: This should be major, not minor. The ObjectError constructor signature changed from (code: string, message: string) to (code: ObjectErrorCode, objectId: string, options: { transportDetails: TransportDetails }) — a source-breaking change for anyone constructing ObjectError directly. The GetObjectsResponse.objects type narrowing and message format changes are also behavioral breaks.

[hayes-mysten]

I skimmed the most recent changes, and not all the things flagged by the review bot are valid, but the current iteration has a ton of real issues. I am not sure this is a good candidate for something to just throw at claude. It's obviously not understanding the problem space well and making a lot of incorrect assumptions. (I wouldn't just go make the changes that the review bot flagged either). I think there is a lot of subtly here that is not trivial to understand.

Is this critical for a project you are working on, or just something you noticed and wanted to improve?

[Danny-Devs]

Edit: Just realized I forgot to answer your question directly — this is just something I noticed and wanted to challenge myself on, not critical for any project of mine.

Hey Hayes — sorry the first pass was so directionally off. I appreciate you taking the time to spell out the principles instead of just closing it.

I took the five points to heart and rebuilt the PR around them. Quick tour of where things landed:

1. Consistent errors across RPCs — ObjectErrorCode is now a transport-agnostic union ('notFound' | 'deleted' | 'unknown'). JSON-RPC distinguishes deleted at the wire level so it maps deleted → 'deleted' and notExists/dynamicFieldNotFound → 'notFound'; gRPC and GraphQL can't distinguish (gRPC's NOT_FOUND is not specific; GraphQL omits absent objects without saying why), so both collapse to 'notFound'. displayError/unknown surface as 'unknown'.

2. Consumers shouldn't need to know which RPC — error.code and error.objectId mean the same thing on all three transports. The fromResponse factory is gone; each transport owns its own mapping.

3. instanceof works across clients — gRPC used to emit plain new Error(...); it now emits ObjectError. GraphQLResponseError now extends SuiClientError (previously extended Error), and the GraphQL multi-error path wraps the aggregate in SuiClientError with transportDetails: { $kind: 'graphql' } on the cause — so instanceof SuiClientError is genuinely universal across all three transports, not just for object errors. core-resolver.ts rethrows the original ObjectError instance when one input is invalid, and throws the new AggregateObjectError extends SuiClientError when more than one is — restoring multi-error reporting that was previously flattened to a concatenated plain Error.

4. Decisions not tied to RPC shape — client/errors.ts no longer imports from ../jsonRpc/. Each transport mapper uses satisfies never so upstream wire additions fail the type-check instead of silently falling through.

5. RPC-specific detail still accessible — TransportDetails is a $kind-tagged union ('jsonRpc' | 'grpc' | 'graphql') on the SuiClientError base. ObjectError's options arg is optional and the field is honestly optional on the base (no declare narrowing on the subclass) — so consumers can construct ObjectError directly without ceremony, while every real SDK construction site still attaches a payload.

Also tightened GetObjectsResponse.objects from (Object | Error)[] → (Object | ObjectError)[] and fixed cross-transport objectId parity (GraphQL now preserves the raw input id).

Let me know what you think about the refactor. Happy to split into smaller PRs if the surface is too big, or iterate further on anything that doesn't match what you had in mind — and of course happy to close it out if it's still not the direction you want.