Skip to content

Commit ec6ab82

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 <99346635+HoberMin@users.noreply.github.com>
1 parent 172f69a commit ec6ab82

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
@@ -69,43 +69,10 @@ describe("InMemoryTransport", () => {
6969
});
7070

7171
test("should throw error when sending after close", async () => {
72-
const [client, server] = InMemoryTransport.createLinkedPair();
73-
let clientError: Error | undefined;
74-
let serverError: Error | undefined;
75-
76-
client.onerror = (err) => {
77-
clientError = err;
78-
};
79-
80-
server.onerror = (err) => {
81-
serverError = err;
82-
};
83-
84-
await client.close();
85-
86-
// Attempt to send message from client
87-
await expect(
88-
client.send({
89-
jsonrpc: "2.0",
90-
method: "test",
91-
id: 1,
92-
}),
93-
).rejects.toThrow("Not connected");
94-
95-
// Attempt to send message from server
72+
await clientTransport.close();
9673
await expect(
97-
server.send({
98-
jsonrpc: "2.0",
99-
method: "test",
100-
id: 2,
101-
}),
74+
clientTransport.send({ jsonrpc: "2.0", method: "test", id: 1 }),
10275
).rejects.toThrow("Not connected");
103-
104-
// Verify that both sides received errors
105-
expect(clientError).toBeDefined();
106-
expect(clientError?.message).toBe("Not connected");
107-
expect(serverError).toBeDefined();
108-
expect(serverError?.message).toBe("Not connected");
10976
});
11077

11178
test("should queue messages sent before start", async () => {
@@ -124,65 +91,4 @@ describe("InMemoryTransport", () => {
12491
await serverTransport.start();
12592
expect(receivedMessage).toEqual(message);
12693
});
127-
128-
describe("error handling", () => {
129-
test("should trigger onerror when sending without connection", async () => {
130-
const transport = new InMemoryTransport();
131-
let error: Error | undefined;
132-
133-
transport.onerror = (err) => {
134-
error = err;
135-
};
136-
137-
await expect(
138-
transport.send({
139-
jsonrpc: "2.0",
140-
method: "test",
141-
id: 1,
142-
}),
143-
).rejects.toThrow("Not connected");
144-
145-
expect(error).toBeDefined();
146-
expect(error?.message).toBe("Not connected");
147-
});
148-
149-
test("should trigger onerror when sending after close", async () => {
150-
const [client, server] = InMemoryTransport.createLinkedPair();
151-
let clientError: Error | undefined;
152-
let serverError: Error | undefined;
153-
154-
client.onerror = (err) => {
155-
clientError = err;
156-
};
157-
158-
server.onerror = (err) => {
159-
serverError = err;
160-
};
161-
162-
await client.close();
163-
164-
// Attempt to send message from client
165-
await expect(
166-
client.send({
167-
jsonrpc: "2.0",
168-
method: "test",
169-
id: 1,
170-
}),
171-
).rejects.toThrow("Not connected");
172-
173-
// Attempt to send message from server
174-
await expect(
175-
server.send({
176-
jsonrpc: "2.0",
177-
method: "test",
178-
id: 2,
179-
}),
180-
).rejects.toThrow("Not connected");
181-
182-
// Verify that both sides received errors
183-
expect(clientError?.message).toBe("Not connected");
184-
expect(serverError).toBeDefined();
185-
expect(serverError?.message).toBe("Not connected");
186-
});
187-
});
18894
});

src/inMemory.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,7 @@ export class InMemoryTransport implements Transport {
4343

4444
async send(message: JSONRPCMessage): Promise<void> {
4545
if (!this._otherTransport) {
46-
const error = new Error("Not connected");
47-
this.onerror?.(error);
48-
throw error;
46+
throw new Error("Not connected");
4947
}
5048

5149
if (this._otherTransport.onmessage) {

0 commit comments

Comments
 (0)