Skip to content

Commit 4b86526

Browse files
authored
fix(client): match subclasses in ORPCError cross-context instanceof check (#1762)
The cross-context `instanceof ORPCError` workaround only checked an instance's direct constructor against the registered constructor set, so instances of classes extending an `ORPCError` from another dependency graph (e.g. Next.js Optimized SSR contexts) failed the check. `Symbol.hasInstance` now walks the instance's whole prototype chain, so such subclass instances are recognized as `ORPCError` too. ## Fixes - Subclasses of a foreign-context `ORPCError` now pass `instanceof ORPCError`; previously only direct instances matched. - Same-context behavior is unchanged — the default `instanceof` fallback and the extended-class guard are untouched, and only registered `ORPCError` constructors can match, so no false positives. ## Testing - New `getConstructors` helper in `@orpc/shared` is covered for primitives, built-ins, inheritance chains, null-prototype objects, and laziness. - New test simulates a foreign dependency graph by registering an unrelated class in the global constructor set, verifying both it and its subclass pass `instanceof ORPCError`, and that they stop matching once deregistered.
1 parent a8d4afc commit 4b86526

4 files changed

Lines changed: 98 additions & 5 deletions

File tree

packages/client/src/error.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,27 @@ describe('oRPCError', () => {
8080
expect(notRelated instanceof NotRelated).toBe(true)
8181
expect(nullProtoObj instanceof NotRelated).toBe(false)
8282
})
83+
84+
it('instanceof matches cross-context ORPCError instances', ({ onTestFinished }) => {
85+
/**
86+
* Stands in for an ORPCError constructor from a separate dependency graph:
87+
* unrelated prototype chain, but registered in the shared global WeakSet.
88+
*/
89+
class CrossContextORPCError extends Error {}
90+
class ExtendedCrossContextORPCError extends CrossContextORPCError {}
91+
92+
expect(new Error('message') instanceof ORPCError).toBe(false)
93+
expect(new CrossContextORPCError() instanceof ORPCError).toBe(false)
94+
expect(new ExtendedCrossContextORPCError() instanceof ORPCError).toBe(false)
95+
96+
const constructors: WeakSet<object> = (globalThis as any)[Symbol.for('ORPC_ERROR_CONSTRUCTORS')]
97+
constructors.add(CrossContextORPCError)
98+
onTestFinished(() => {
99+
constructors.delete(CrossContextORPCError)
100+
})
101+
102+
expect(new Error('message') instanceof ORPCError).toBe(false)
103+
expect(new CrossContextORPCError() instanceof ORPCError).toBe(true)
104+
expect(new ExtendedCrossContextORPCError() instanceof ORPCError).toBe(true)
105+
})
83106
})

packages/client/src/error.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { MaybeOptionalOptions, Registry } from '@orpc/shared'
2-
import { getConstructor, resolveMaybeOptionalOptions } from '@orpc/shared'
2+
import { getConstructors, resolveMaybeOptionalOptions } from '@orpc/shared'
33

44
export const COMMON_ERROR_STATUS_MAP = {
55
BAD_REQUEST: 400,
@@ -125,9 +125,10 @@ export class ORPCError<TCode extends ORPCErrorCode, TData> extends Error {
125125
return super[Symbol.hasInstance](instance)
126126
}
127127

128-
const constructor = getConstructor(instance)
129-
if (constructor && ORPCErrorConstructors.has(constructor)) {
130-
return true
128+
for (const constructor of getConstructors(instance)) {
129+
if (ORPCErrorConstructors.has(constructor)) {
130+
return true
131+
}
131132
}
132133

133134
// fallback to default instanceof check

packages/shared/src/object.test.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as a from 'arktype'
22
import * as v from 'valibot'
33
import z from 'zod'
4-
import { bindMethods, clone, findDeepMatches, get, getConstructor, isPlainObject, isPropertyKey, NullProtoObj, omit, set } from './object'
4+
import { bindMethods, clone, findDeepMatches, get, getConstructor, getConstructors, isPlainObject, isPropertyKey, NullProtoObj, omit, set } from './object'
55

66
it('findDeepMatches', () => {
77
const { maps, values } = findDeepMatches(v => typeof v === 'string', {
@@ -43,6 +43,61 @@ it('getConstructor', () => {
4343
expect(getConstructor(() => { })).toBe(Function)
4444
})
4545

46+
describe('getConstructors', () => {
47+
it('returns nothing for primitives', () => {
48+
expect([...getConstructors(null)]).toEqual([])
49+
expect([...getConstructors(undefined)]).toEqual([])
50+
expect([...getConstructors(123)]).toEqual([])
51+
expect([...getConstructors('string')]).toEqual([])
52+
expect([...getConstructors(true)]).toEqual([])
53+
expect([...getConstructors(Symbol('s'))]).toEqual([])
54+
})
55+
56+
it('works with plain objects and built-ins', () => {
57+
expect([...getConstructors({})]).toEqual([Object])
58+
expect([...getConstructors([])]).toEqual([Array, Object])
59+
expect([...getConstructors(new Map())]).toEqual([Map, Object])
60+
expect([...getConstructors(new Error('hi'))]).toEqual([Error, Object])
61+
})
62+
63+
it('returns the full chain for class inheritance', () => {
64+
class GrandParent {}
65+
class Parent extends GrandParent {}
66+
class Child extends Parent {}
67+
68+
expect([...getConstructors(new Child())]).toEqual([Child, Parent, GrandParent, Object])
69+
expect([...getConstructors(new Parent())]).toEqual([Parent, GrandParent, Object])
70+
expect([...getConstructors(new GrandParent())]).toEqual([GrandParent, Object])
71+
})
72+
73+
it('works with classes extending built-ins', () => {
74+
class MyError extends Error {}
75+
class SubError extends MyError {}
76+
77+
expect([...getConstructors(new SubError())]).toEqual([SubError, MyError, Error, Object])
78+
})
79+
80+
it('returns nothing for null-prototype objects', () => {
81+
expect([...getConstructors(Object.create(null))]).toEqual([])
82+
})
83+
84+
it('skips levels without a constructor', () => {
85+
const proto = Object.create(null) // no constructor
86+
const instance = Object.create(proto)
87+
88+
expect([...getConstructors(instance)]).toEqual([])
89+
})
90+
91+
it('is lazy', () => {
92+
class Parent {}
93+
class Child extends Parent {}
94+
95+
const generator = getConstructors(new Child())
96+
expect(generator.next().value).toBe(Child)
97+
expect(generator.next().value).toBe(Parent)
98+
})
99+
})
100+
46101
it('isPlainObject', () => {
47102
expect(new Error('hi')).not.toSatisfy(isPlainObject)
48103
expect(new Map()).not.toSatisfy(isPlainObject)

packages/shared/src/object.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ export function getConstructor(value: unknown): Function | null | undefined { //
4141
return Object.getPrototypeOf(value)?.constructor
4242
}
4343

44+
export function* getConstructors(value: unknown): Generator<Function> { // eslint-disable-line ts/no-unsafe-function-type
45+
if (!isTypescriptObject(value)) {
46+
return
47+
}
48+
49+
let proto = Object.getPrototypeOf(value)
50+
while (proto != null) {
51+
if (proto.constructor) {
52+
yield proto.constructor
53+
}
54+
proto = Object.getPrototypeOf(proto)
55+
}
56+
}
57+
4458
/**
4559
* Checks whether a value is a plain object, including objects created with
4660
* `Object.create(null)`.

0 commit comments

Comments
 (0)