Skip to content

Commit

Permalink
Fix error serialization for rpc calls (#1278)
Browse files Browse the repository at this point in the history
  • Loading branch information
jerelmiller authored Mar 20, 2024
1 parent cc98a1e commit e886145
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/hip-dots-worry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"apollo-client-devtools": patch
---

Fix issue with error serialization when sending an error back through the message passing system. Unfortunately the raw error instance was lost in this process. This fix retains the error message when sending error messages in rpc calls.
44 changes: 29 additions & 15 deletions src/extension/__tests__/rpc.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { DistributiveOmit } from "../../types";
import { RPC_MESSAGE_TIMEOUT } from "../errorMessages";
import type { MessageAdapter } from "../messageAdapters";
import type { RPCMessage, RPCRequestMessage } from "../messages";
import { MessageType } from "../messages";
Expand Down Expand Up @@ -170,6 +171,29 @@ test("rejects when async handler rejects", async () => {
);
});

test("maintains error name", async () => {
type Message = {
add(x: number, y: number): number;
};

const handlerAdapter = createTestAdapter();
const clientAdapter = createTestAdapter();
createBridge(clientAdapter, handlerAdapter);

const client = createRpcClient<Message>(clientAdapter);
const handle = createRpcHandler<Message>(handlerAdapter);

handle("add", () => Promise.reject(new SyntaxError()));

try {
await client.request("add", 1, 2);
throw new Error("Should not reach");
} catch (e) {
expect(e).toBeInstanceOf(SyntaxError);
expect((e as Error).name).toBe("SyntaxError");
}
});

test("can handle multiple rpc messages", async () => {
type Message = {
add(x: number, y: number): number;
Expand Down Expand Up @@ -403,9 +427,7 @@ test("times out if no message received within default timeout", async () => {

jest.advanceTimersByTime(30_000);

await expect(promise).rejects.toEqual(
new Error("Timeout waiting for message")
);
await expect(promise).rejects.toEqual(new Error(RPC_MESSAGE_TIMEOUT));

jest.useRealTimers();
});
Expand All @@ -423,9 +445,7 @@ test("times out if no message received within configured timeout", async () => {

jest.advanceTimersByTime(1000);

await expect(promise).rejects.toEqual(
new Error("Timeout waiting for message")
);
await expect(promise).rejects.toEqual(new Error(RPC_MESSAGE_TIMEOUT));

jest.useRealTimers();
});
Expand Down Expand Up @@ -469,17 +489,13 @@ test("resets timeout to default timeout after sending request", async () => {
expect(finished).toContain(promise1);
expect(finished).not.toContain(promise2);

await expect(promise1).rejects.toEqual(
new Error("Timeout waiting for message")
);
await expect(promise1).rejects.toEqual(new Error(RPC_MESSAGE_TIMEOUT));

jest.advanceTimersByTime(1000);
// wait for a tick, just to be sure the `finally` block has time to run
await Promise.resolve();
expect(finished).toContain(promise2);
await expect(promise2).rejects.toEqual(
new Error("Timeout waiting for message")
);
await expect(promise2).rejects.toEqual(new Error(RPC_MESSAGE_TIMEOUT));

const promise3 = client.request("add", 1, 2);
promise3
Expand All @@ -499,9 +515,7 @@ test("resets timeout to default timeout after sending request", async () => {
// wait for a tick, just to be sure the `finally` block has time to run
await Promise.resolve();
expect(finished).toContain(promise3);
await expect(promise3).rejects.toEqual(
new Error("Timeout waiting for message")
);
await expect(promise3).rejects.toEqual(new Error(RPC_MESSAGE_TIMEOUT));

jest.useRealTimers();
});
Expand Down
1 change: 1 addition & 0 deletions src/extension/errorMessages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const RPC_MESSAGE_TIMEOUT = "RPC_MESSAGE_TIMEOUT";
40 changes: 40 additions & 0 deletions src/extension/errorSerialization.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const errorConstructors = [
EvalError,
RangeError,
ReferenceError,
SyntaxError,
TypeError,
URIError,
].reduce(
(memo, constructor) => memo.set(constructor.name, constructor),
new Map<string, ErrorConstructor>()
);

export function serializeError(error: unknown) {
return error instanceof Error
? { name: error.name, message: error.message, stack: error.stack }
: { message: String(error) };
}

export function deserializeError({
name,
message,
stack,
}: {
name?: string;
message: string;
stack?: string;
}) {
const ErrorClass = name ? errorConstructors.get(name) ?? Error : Error;
const error = new ErrorClass(message);

if (name && error.name !== name) {
error.name = name;
}

if (stack) {
error.stack = stack;
}

return error;
}
2 changes: 1 addition & 1 deletion src/extension/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export type RPCErrorResponseMessage = {
type: MessageType.RPCResponse;
id: string;
sourceId: string;
error: unknown;
error: { name?: string; message: string; stack?: string };
};

export type RPCSuccessResponseMessage<Result = unknown> = {
Expand Down
10 changes: 6 additions & 4 deletions src/extension/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { NoInfer, SafeAny } from "../types";
import { RPC_MESSAGE_TIMEOUT } from "./errorMessages";
import { deserializeError, serializeError } from "./errorSerialization";
import type { MessageAdapter } from "./messageAdapters";
import type {
RPCMessage,
Expand Down Expand Up @@ -39,7 +41,7 @@ export function createRpcClient<Messages extends MessageCollection>(

const timeout = setTimeout(() => {
removeListener();
reject(new Error("Timeout waiting for message"));
reject(new Error(RPC_MESSAGE_TIMEOUT));
}, this.timeout);

const removeListener = adapter.addListener((message) => {
Expand All @@ -48,7 +50,7 @@ export function createRpcClient<Messages extends MessageCollection>(
}

if ("error" in message) {
reject(message.error);
reject(deserializeError(message.error));
} else {
resolve(message.result);
}
Expand Down Expand Up @@ -119,13 +121,13 @@ export function createRpcHandler<Messages extends MessageCollection>(
sourceId: id,
result,
});
} catch (error) {
} catch (e) {
adapter.postMessage({
source: "apollo-client-devtools",
type: MessageType.RPCResponse,
id: createId(),
sourceId: id,
error,
error: serializeError(e),
});
}
});
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"allowJs": true,
"sourceMap": true,
"noImplicitAny": true,
"strictNullChecks": true,
Expand Down

0 comments on commit e886145

Please sign in to comment.