Skip to content

Commit 9a22a16

Browse files
fix(core): use SdkError for duplicate extension; defer capability check pre-connect
- duplicate-ID guard now throws SdkError(ExtensionAlreadyRegistered) instead of plain Error - _assertPeerCapability defers when peer capabilities are not yet populated, so strict-mode sendRequest before connect() surfaces NotConnected (from the underlying send) rather than a misleading CapabilityNotSupported
1 parent 6cb37f3 commit 9a22a16

5 files changed

Lines changed: 30 additions & 6 deletions

File tree

packages/client/src/client/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,15 @@ export class Client extends Protocol<ClientContext> {
340340
throw new SdkError(SdkErrorCode.AlreadyConnected, 'Cannot register extension after connecting to transport');
341341
}
342342
if (this._capabilities.extensions && Object.hasOwn(this._capabilities.extensions, id)) {
343-
throw new Error(`Extension "${id}" is already registered`);
343+
throw new SdkError(SdkErrorCode.ExtensionAlreadyRegistered, `Extension "${id}" is already registered`);
344344
}
345345
this._capabilities.extensions = { ...this._capabilities.extensions, [id]: settings };
346346
return new ExtensionHandle(
347347
this,
348348
id,
349349
settings,
350350
() => this._serverCapabilities?.extensions?.[id],
351+
() => this._serverCapabilities !== undefined,
351352
() => this._enforceStrictCapabilities,
352353
opts?.peerSchema
353354
);

packages/core/src/errors/sdkErrors.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export enum SdkErrorCode {
1010
// State errors
1111
/** Transport is not connected */
1212
NotConnected = 'NOT_CONNECTED',
13+
ExtensionAlreadyRegistered = 'EXTENSION_ALREADY_REGISTERED',
1314
/** Transport is already connected */
1415
AlreadyConnected = 'ALREADY_CONNECTED',
1516
/** Protocol is not initialized */

packages/core/src/shared/extensionHandle.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export class ExtensionHandle<Local extends JSONObject, Peer = JSONObject, Contex
6767
/** The local settings object advertised in `capabilities.extensions[id]`. */
6868
public readonly settings: Local,
6969
private readonly _getPeerExtensionSettings: () => JSONObject | undefined,
70+
private readonly _getPeerCapabilitiesPresent: () => boolean,
7071
private readonly _getEnforceStrictCapabilities: () => boolean,
7172
private readonly _peerSchema?: AnySchema
7273
) {}
@@ -150,6 +151,10 @@ export class ExtensionHandle<Local extends JSONObject, Peer = JSONObject, Contex
150151
}
151152

152153
private _assertPeerCapability(method: string): void {
154+
// If peer capabilities are not yet populated (pre-connect), defer to the
155+
// NotConnected error from the underlying send path rather than misreporting
156+
// CapabilityNotSupported.
157+
if (!this._getPeerCapabilitiesPresent()) return;
153158
if (this._getEnforceStrictCapabilities() && this._getPeerExtensionSettings() === undefined) {
154159
throw new SdkError(
155160
SdkErrorCode.CapabilityNotSupported,

packages/core/test/shared/extensionHandle.test.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ function makeMockHost(): MockHost {
2323
};
2424
}
2525

26-
function makeHandle(opts: { peer?: JSONObject | undefined; strict?: boolean; peerSchema?: z.core.$ZodType }): {
26+
function makeHandle(opts: { peer?: JSONObject | undefined; peerPresent?: boolean; strict?: boolean; peerSchema?: z.core.$ZodType }): {
2727
host: MockHost;
2828
handle: ExtensionHandle<JSONObject, unknown, BaseContext>;
2929
} {
@@ -33,6 +33,7 @@ function makeHandle(opts: { peer?: JSONObject | undefined; strict?: boolean; pee
3333
'io.example/ui',
3434
{ local: true },
3535
() => opts.peer,
36+
() => opts.peerPresent ?? opts.peer !== undefined,
3637
() => opts.strict ?? false,
3738
opts.peerSchema
3839
);
@@ -70,7 +71,14 @@ describe('ExtensionHandle.getPeerSettings', () => {
7071
let peer: JSONObject | undefined;
7172
const getter = vi.fn(() => peer);
7273
const host = makeMockHost() as unknown as ExtensionHost<BaseContext>;
73-
const handle = new ExtensionHandle(host, 'io.example/ui', {}, getter, () => false);
74+
const handle = new ExtensionHandle(
75+
host,
76+
'io.example/ui',
77+
{},
78+
getter,
79+
() => true,
80+
() => false
81+
);
7482

7583
expect(handle.getPeerSettings()).toBeUndefined();
7684
peer = { v: 1 };
@@ -102,15 +110,15 @@ describe('ExtensionHandle.sendRequest / sendNotification — peer gating', () =>
102110
const Result = z.object({ ok: z.boolean() });
103111

104112
test('lax mode (default): sends even when peer did not advertise', async () => {
105-
const { host, handle } = makeHandle({ peer: undefined, strict: false });
113+
const { host, handle } = makeHandle({ peer: undefined, peerPresent: true, strict: false });
106114
await handle.sendRequest('ui/do', { x: 1 }, Result);
107115
expect(host.sendCustomRequest).toHaveBeenCalledWith('ui/do', { x: 1 }, Result, undefined);
108116
await handle.sendNotification('ui/ping', {});
109117
expect(host.sendCustomNotification).toHaveBeenCalledWith('ui/ping', {}, undefined);
110118
});
111119

112120
test('strict mode: rejects with CapabilityNotSupported when peer did not advertise', async () => {
113-
const { host, handle } = makeHandle({ peer: undefined, strict: true });
121+
const { host, handle } = makeHandle({ peer: undefined, peerPresent: true, strict: true });
114122
await expect(handle.sendRequest('ui/do', {}, Result)).rejects.toSatisfy(
115123
(e: unknown) =>
116124
e instanceof SdkError && e.code === SdkErrorCode.CapabilityNotSupported && /io\.example\/ui.*ui\/do/.test(e.message)
@@ -138,3 +146,11 @@ describe('ExtensionHandle — id and settings', () => {
138146
expect(handle.settings).toEqual({ local: true });
139147
});
140148
});
149+
150+
describe('ExtensionHandle — pre-connect strict mode defers to NotConnected', () => {
151+
test('does not throw CapabilityNotSupported before peer capabilities are known', async () => {
152+
const { host, handle } = makeHandle({ peer: undefined, peerPresent: false, strict: true });
153+
await handle.sendRequest('ui/do', {}, z.object({}));
154+
expect(host.sendCustomRequest).toHaveBeenCalledOnce();
155+
});
156+
});

packages/server/src/server/server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,15 @@ export class Server extends Protocol<ServerContext> {
254254
throw new SdkError(SdkErrorCode.AlreadyConnected, 'Cannot register extension after connecting to transport');
255255
}
256256
if (this._capabilities.extensions && Object.hasOwn(this._capabilities.extensions, id)) {
257-
throw new Error(`Extension "${id}" is already registered`);
257+
throw new SdkError(SdkErrorCode.ExtensionAlreadyRegistered, `Extension "${id}" is already registered`);
258258
}
259259
this._capabilities.extensions = { ...this._capabilities.extensions, [id]: settings };
260260
return new ExtensionHandle(
261261
this,
262262
id,
263263
settings,
264264
() => this._clientCapabilities?.extensions?.[id],
265+
() => this._clientCapabilities !== undefined,
265266
() => this._enforceStrictCapabilities,
266267
opts?.peerSchema
267268
);

0 commit comments

Comments
 (0)