Skip to content

Commit 88ede37

Browse files
Address review feedback: move getDefaultServerParameters to test file
- Moved getDefaultServerParameters from stdio.ts to stdio.test.ts since it's only used in tests - Reverted unrelated changes to inMemory.ts and inMemory.test.ts - Kept the core Windows compatibility fix for stdio tests Co-authored-by: HoberMin <[email protected]>
1 parent a7dc125 commit 88ede37

File tree

4 files changed

+13
-109
lines changed

4 files changed

+13
-109
lines changed

src/client/stdio.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
import { JSONRPCMessage } from "../types.js";
2-
import { StdioClientTransport, getDefaultServerParameters } from "./stdio.js";
2+
import { StdioClientTransport, StdioServerParameters } from "./stdio.js";
3+
4+
// Configure default server parameters based on OS
5+
// Uses 'more' command for Windows and 'tee' command for Unix/Linux
6+
const getDefaultServerParameters = (): StdioServerParameters => {
7+
if (process.platform === "win32") {
8+
return { command: "more" };
9+
}
10+
return { command: "/usr/bin/tee" };
11+
};
312

413
const serverParameters = getDefaultServerParameters();
514

src/client/stdio.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,6 @@ export type StdioServerParameters = {
3939
cwd?: string;
4040
};
4141

42-
// Configure default server parameters based on OS
43-
// Uses 'more' command for Windows and 'tee' command for Unix/Linux
44-
export const getDefaultServerParameters = (): StdioServerParameters => {
45-
if (process.platform === "win32") {
46-
return { command: "more" };
47-
}
48-
return { command: "/usr/bin/tee" };
49-
};
50-
5142
/**
5243
* Environment variables to inherit by default, if an environment is not explicitly given.
5344
*/

src/inMemory.test.ts

Lines changed: 2 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -96,43 +96,10 @@ describe("InMemoryTransport", () => {
9696
});
9797

9898
test("should throw error when sending after close", async () => {
99-
const [client, server] = InMemoryTransport.createLinkedPair();
100-
let clientError: Error | undefined;
101-
let serverError: Error | undefined;
102-
103-
client.onerror = (err) => {
104-
clientError = err;
105-
};
106-
107-
server.onerror = (err) => {
108-
serverError = err;
109-
};
110-
111-
await client.close();
112-
113-
// Attempt to send message from client
114-
await expect(
115-
client.send({
116-
jsonrpc: "2.0",
117-
method: "test",
118-
id: 1,
119-
}),
120-
).rejects.toThrow("Not connected");
121-
122-
// Attempt to send message from server
99+
await clientTransport.close();
123100
await expect(
124-
server.send({
125-
jsonrpc: "2.0",
126-
method: "test",
127-
id: 2,
128-
}),
101+
clientTransport.send({ jsonrpc: "2.0", method: "test", id: 1 }),
129102
).rejects.toThrow("Not connected");
130-
131-
// Verify that both sides received errors
132-
expect(clientError).toBeDefined();
133-
expect(clientError?.message).toBe("Not connected");
134-
expect(serverError).toBeDefined();
135-
expect(serverError?.message).toBe("Not connected");
136103
});
137104

138105
test("should queue messages sent before start", async () => {
@@ -151,65 +118,4 @@ describe("InMemoryTransport", () => {
151118
await serverTransport.start();
152119
expect(receivedMessage).toEqual(message);
153120
});
154-
155-
describe("error handling", () => {
156-
test("should trigger onerror when sending without connection", async () => {
157-
const transport = new InMemoryTransport();
158-
let error: Error | undefined;
159-
160-
transport.onerror = (err) => {
161-
error = err;
162-
};
163-
164-
await expect(
165-
transport.send({
166-
jsonrpc: "2.0",
167-
method: "test",
168-
id: 1,
169-
}),
170-
).rejects.toThrow("Not connected");
171-
172-
expect(error).toBeDefined();
173-
expect(error?.message).toBe("Not connected");
174-
});
175-
176-
test("should trigger onerror when sending after close", async () => {
177-
const [client, server] = InMemoryTransport.createLinkedPair();
178-
let clientError: Error | undefined;
179-
let serverError: Error | undefined;
180-
181-
client.onerror = (err) => {
182-
clientError = err;
183-
};
184-
185-
server.onerror = (err) => {
186-
serverError = err;
187-
};
188-
189-
await client.close();
190-
191-
// Attempt to send message from client
192-
await expect(
193-
client.send({
194-
jsonrpc: "2.0",
195-
method: "test",
196-
id: 1,
197-
}),
198-
).rejects.toThrow("Not connected");
199-
200-
// Attempt to send message from server
201-
await expect(
202-
server.send({
203-
jsonrpc: "2.0",
204-
method: "test",
205-
id: 2,
206-
}),
207-
).rejects.toThrow("Not connected");
208-
209-
// Verify that both sides received errors
210-
expect(clientError?.message).toBe("Not connected");
211-
expect(serverError).toBeDefined();
212-
expect(serverError?.message).toBe("Not connected");
213-
});
214-
});
215121
});

src/inMemory.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,7 @@ export class InMemoryTransport implements Transport {
5151
*/
5252
async send(message: JSONRPCMessage, options?: { relatedRequestId?: RequestId, authInfo?: AuthInfo }): Promise<void> {
5353
if (!this._otherTransport) {
54-
const error = new Error("Not connected");
55-
this.onerror?.(error);
56-
throw error;
54+
throw new Error("Not connected");
5755
}
5856

5957
if (this._otherTransport.onmessage) {

0 commit comments

Comments
 (0)