Skip to content

sdk: properly reject pending agent-ready promises and surface structured server errors #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
willwashburn merged 3 commits into from
Feb 8, 2026
Merged

sdk: properly reject pending agent-ready promises and surface structured server errors #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1978ece
Initial plan
Copilot Feb 8, 2026
54cef38
Address PR review comments: reject pendingAgentReady properly, add Re…
Copilot Feb 8, 2026
158dff3
Add packages/*/package-lock.json to .gitignore and remove tracked files
Copilot Feb 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Dependencies
node_modules/
packages/*/package-lock.json

# Build output
dist/
Expand Down
33 changes: 33 additions & 0 deletions packages/sdk/src/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
SpawnResultPayload,
} from '@agent-relay/protocol';
import { RelayClient } from './client.js';
import { RelayServerError } from './errors.js';

describe('RelayClient', () => {
describe('configuration', () => {
Expand Down Expand Up @@ -259,6 +260,38 @@ describe('RelayClient', () => {
expect((client as any).pendingSpawns.size).toBe(0);
expect((client as any).pendingAgentReady.size).toBe(0);
});

it('invokes onError callback with structured error for server ERROR frames', async () => {
const onErrorMock = vi.fn();
const client = new RelayClient({
reconnect: false,
quiet: true,
});
client.onError = onErrorMock;
(client as any)._state = 'READY';

const errorEnvelope: Envelope<ErrorPayload> = {
v: 1,
type: 'ERROR',
id: 'err-test-structured',
ts: Date.now(),
payload: {
code: 'UNAUTHORIZED' as any,
message: 'Authentication failed',
fatal: false,
},
};

(client as any).processFrame(errorEnvelope);

expect(onErrorMock).toHaveBeenCalledTimes(1);
const errorArg = onErrorMock.mock.calls[0][0];
expect(errorArg).toBeInstanceOf(RelayServerError);
expect(errorArg.message).toBe('Authentication failed');
expect(errorArg.code).toBe('UNAUTHORIZED');
expect(errorArg.fatal).toBe(false);
expect(errorArg.envelope).toBe(errorEnvelope);
});
});

describe('sendMessage', () => {
Expand Down
129 changes: 70 additions & 59 deletions packages/sdk/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import net from 'node:net';
import { randomUUID } from 'node:crypto';
import { discoverSocket } from '@agent-relay/utils/discovery';
import { DaemonNotRunningError, ConnectionError } from '@agent-relay/utils/errors';
import { RelayServerError } from './errors.js';
// Import shared protocol types and framing utilities from @agent-relay/protocol
import {
type Envelope,
Expand Down Expand Up @@ -830,60 +831,71 @@ export class RelayClient {
}

// Send the spawn request
const spawnResult = await new Promise<SpawnResultPayload>((resolve, reject) => {
const timeoutHandle = setTimeout(() => {
this.pendingSpawns.delete(envelopeId);
// Also clean up pending agent ready if spawn times out
if (waitForReady) {
const pending = this.pendingAgentReady.get(options.name);
if (pending) {
clearTimeout(pending.timeoutHandle);
this.pendingAgentReady.delete(options.name);
let spawnResult: SpawnResultPayload;
try {
spawnResult = await new Promise<SpawnResultPayload>((resolve, reject) => {
const timeoutHandle = setTimeout(() => {
this.pendingSpawns.delete(envelopeId);
// Also clean up pending agent ready if spawn times out
if (waitForReady) {
const pending = this.pendingAgentReady.get(options.name);
if (pending) {
clearTimeout(pending.timeoutHandle);
this.pendingAgentReady.delete(options.name);
}
}
}
reject(new Error(`Spawn timeout after ${timeoutMs}ms`));
}, timeoutMs);

this.pendingSpawns.set(envelopeId, { resolve, reject, timeoutHandle });

const envelope: SpawnEnvelope = {
v: PROTOCOL_VERSION,
type: 'SPAWN',
id: envelopeId,
ts: Date.now(),
payload: {
name: options.name,
cli: options.cli,
task: options.task || '',
cwd: options.cwd,
team: options.team,
interactive: options.interactive,
shadowMode: options.shadowMode,
shadowOf: options.shadowOf,
shadowAgent: options.shadowAgent,
shadowTriggers: options.shadowTriggers,
shadowSpeakOn: options.shadowSpeakOn,
userId: options.userId,
includeWorkflowConventions: options.includeWorkflowConventions,
spawnerName: options.spawnerName || this.config.agentName,
},
};

const sent = this.send(envelope);
if (!sent) {
clearTimeout(timeoutHandle);
this.pendingSpawns.delete(envelopeId);
// Also clean up pending agent ready if send fails
if (waitForReady) {
const pending = this.pendingAgentReady.get(options.name);
if (pending) {
clearTimeout(pending.timeoutHandle);
this.pendingAgentReady.delete(options.name);
reject(new Error(`Spawn timeout after ${timeoutMs}ms`));
}, timeoutMs);

this.pendingSpawns.set(envelopeId, { resolve, reject, timeoutHandle });

const envelope: SpawnEnvelope = {
v: PROTOCOL_VERSION,
type: 'SPAWN',
id: envelopeId,
ts: Date.now(),
payload: {
name: options.name,
cli: options.cli,
task: options.task || '',
cwd: options.cwd,
team: options.team,
interactive: options.interactive,
shadowMode: options.shadowMode,
shadowOf: options.shadowOf,
shadowAgent: options.shadowAgent,
shadowTriggers: options.shadowTriggers,
shadowSpeakOn: options.shadowSpeakOn,
userId: options.userId,
includeWorkflowConventions: options.includeWorkflowConventions,
spawnerName: options.spawnerName || this.config.agentName,
},
};

const sent = this.send(envelope);
if (!sent) {
clearTimeout(timeoutHandle);
this.pendingSpawns.delete(envelopeId);
// Also clean up pending agent ready if send fails
if (waitForReady) {
const pending = this.pendingAgentReady.get(options.name);
if (pending) {
clearTimeout(pending.timeoutHandle);
this.pendingAgentReady.delete(options.name);
}
}
reject(new Error('Failed to send spawn message'));
}
reject(new Error('Failed to send spawn message'));
});
} catch (err) {
// Spawn request failed - suppress any pending readyPromise rejection
if (readyPromise) {
readyPromise.catch(() => {
// Suppress unhandled rejection from readyPromise
});
}
});
throw err;
}

// If spawn failed or we don't need to wait for ready, return immediately
if (!spawnResult.success || !waitForReady || !readyPromise) {
Expand Down Expand Up @@ -1862,7 +1874,7 @@ export class RelayClient {
this.rejectPendingReleases(err);
this.rejectPendingSendInputs(err);
this.rejectPendingListWorkers(err);
this.clearPendingAgentReady();
this.rejectPendingAgentReady(err);
}

if (envelope.payload.code === 'RESUME_TOO_OLD') {
Expand All @@ -1880,7 +1892,13 @@ export class RelayClient {

// Surface server-side ERROR frames to SDK consumers.
if (this.onError) {
this.onError(new Error(errorMessage));
const serverError = new RelayServerError(
errorMessage,
envelope.payload.code,
envelope.payload.fatal,
envelope
);
this.onError(serverError);
}
}

Expand Down Expand Up @@ -1986,13 +2004,6 @@ export class RelayClient {
}
}

private clearPendingAgentReady(): void {
for (const [agentName, pending] of this.pendingAgentReady.entries()) {
clearTimeout(pending.timeoutHandle);
this.pendingAgentReady.delete(agentName);
}
}

private scheduleReconnect(): void {
this.setState('BACKOFF');
this.reconnectAttempts++;
Expand Down
17 changes: 17 additions & 0 deletions packages/sdk/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,20 @@ export {
ChannelNotFoundError,
SpawnError,
} from '@agent-relay/utils/errors';

// RelayServerError is defined locally for SDK-specific error handling
import { RelayError } from '@agent-relay/utils/errors';

export class RelayServerError extends RelayError {
code: string;
fatal: boolean;
envelope?: any;

constructor(message: string, code: string, fatal: boolean, envelope?: any) {
super(message);
this.name = 'RelayServerError';
this.code = code;
this.fatal = fatal;
this.envelope = envelope;
}
}
Comment on lines +22 to +34
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Duplicate RelayServerError class definitions cause instanceof failures across packages

RelayServerError is defined as two separate classes: one in packages/utils/src/errors.ts:58-70 and another in packages/sdk/src/errors.ts:22-34. The SDK's client.ts imports from ./errors.js (the SDK-local copy), but @agent-relay/utils/errors also exports its own RelayServerError.

Root Cause and Impact

The established pattern in this codebase is that @agent-relay/utils/errors is the "single source of truth" for error classes (as stated in the file header comments at packages/sdk/src/errors.ts:4-6). All other error classes in the SDK's errors.ts are re-exported from @agent-relay/utils/errors. However, RelayServerError breaks this pattern by being defined locally in the SDK while also being added to @agent-relay/utils/errors.

Because these are two distinct JavaScript class constructors, instanceof checks will fail when comparing across packages. For example, if any code in the @agent-relay/utils or @agent-relay/daemon packages creates a RelayServerError using their import from @agent-relay/utils/errors, and SDK consumer code checks error instanceof RelayServerError using the SDK's version, the check will return false.

The SDK's errors.ts should re-export RelayServerError from @agent-relay/utils/errors instead of defining its own copy, consistent with how all other error classes are handled.

Prompt for agents 
Remove the local RelayServerError class definition from packages/sdk/src/errors.ts (lines 19-34) and instead re-export it from @agent-relay/utils/errors, consistent with how all other error classes are handled. Change the export block at lines 9-17 to include RelayServerError:

export {
  RelayError,
  DaemonNotRunningError,
  AgentNotFoundError,
  TimeoutError,
  ConnectionError,
  ChannelNotFoundError,
  SpawnError,
  RelayServerError,
} from '@agent-relay/utils/errors';

Also remove the now-unnecessary local import of RelayError on line 20 and the entire local class definition on lines 22-34.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

14 changes: 14 additions & 0 deletions packages/utils/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,17 @@ export class SpawnError extends RelayError {
this.name = 'SpawnError';
}
}

export class RelayServerError extends RelayError {
code: string;
fatal: boolean;
envelope?: any;

constructor(message: string, code: string, fatal: boolean, envelope?: any) {
super(message);
this.name = 'RelayServerError';
this.code = code;
this.fatal = fatal;
this.envelope = envelope;
}
}