Skip to content

Commit 365de24

Browse files
NathanFlurryNathanFlurry
committed
chore: add explicit error handling for all hono routes (#702)
1 parent 727dd28 commit 365de24

File tree

7 files changed

+228
-169
lines changed

7 files changed

+228
-169
lines changed

‎packages/actor-core/src/actor/protocol/message/mod.ts

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
InputData,
1414
CachedSerializer,
1515
} from "@/actor/protocol/serde";
16+
import { deconstructError } from "@/common/utils";
1617

1718
export const TransportSchema = z.enum(["websocket", "sse"]);
1819

@@ -132,38 +133,11 @@ export async function processMessage<A extends AnyActor>(
132133
assertUnreachable(message.b);
133134
}
134135
} catch (error) {
135-
// Build response error information. Only return errors if flagged as public in order to prevent leaking internal behavior.
136-
//
137-
// We log the error here instead of after generating the code & message because we need to log the original error, not the masked internal error.
138-
let code: string;
139-
let message: string;
140-
let metadata: unknown = undefined;
141-
if (error instanceof errors.ActorError && error.public) {
142-
code = error.code;
143-
message = String(error);
144-
metadata = error.metadata;
145-
146-
logger().info("public error", {
147-
code,
148-
message,
149-
connectionId: conn.id,
150-
rpcId,
151-
rpcName,
152-
});
153-
} else {
154-
code = errors.INTERNAL_ERROR_CODE;
155-
message = errors.INTERNAL_ERROR_DESCRIPTION;
156-
metadata = {
157-
//url: `https://hub.rivet.gg/projects/${actorMetadata.project.slug}/environments/${actorMetadata.environment.slug}/actors?actorId=${actorMetadata.actor.id}`,
158-
} satisfies errors.InternalErrorMetadata;
159-
160-
logger().warn("internal error", {
161-
error: String(error),
162-
connectionId: conn.id,
163-
rpcId,
164-
rpcName,
165-
});
166-
}
136+
const { code, message, metadata } = deconstructError(error, logger(), {
137+
connectionId: conn.id,
138+
rpcId,
139+
rpcName,
140+
});
167141

168142
// Build response
169143
if (rpcId !== undefined) {

‎packages/actor-core/src/actor/runtime/actor_router.ts

Lines changed: 95 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { SSEStreamingApi, streamSSE } from "hono/streaming";
1313
import { cors } from "hono/cors";
1414
import { assertUnreachable } from "./utils";
1515
import { createInspectorRouter, InspectorConnectionHandler } from "./inspect";
16+
import { handleRouteError, handleRouteNotFound } from "@/common/router";
17+
import { deconstructError } from "@/common/utils";
1618

1719
export interface ConnectWebSocketOpts {
1820
req: HonoRequest;
@@ -98,58 +100,85 @@ export function createActorRouter(
98100
app.get(
99101
"/connect/websocket",
100102
handler.upgradeWebSocket(async (c) => {
101-
if (!handler.onConnectWebSocket)
102-
throw new Error("onConnectWebSocket is not implemented");
103-
104-
const encoding = getRequestEncoding(c.req);
105-
const parameters = getRequestConnectionParameters(c.req, config);
106-
107-
const wsHandler = await handler.onConnectWebSocket({
108-
req: c.req,
109-
encoding,
110-
parameters,
111-
});
112-
113-
const { promise: onOpenPromise, resolve: onOpenResolve } =
114-
Promise.withResolvers<undefined>();
115-
return {
116-
onOpen: async (_evt, ws) => {
117-
logger().debug("websocket open");
118-
119-
// Call handler
120-
await wsHandler.onOpen(ws);
121-
122-
// Resolve promise
123-
onOpenResolve(undefined);
124-
},
125-
onMessage: async (evt) => {
126-
await onOpenPromise;
127-
128-
logger().debug("received message");
129-
130-
const value = evt.data.valueOf() as InputData;
131-
const message = await parseMessage(value, {
132-
encoding: encoding,
133-
maxIncomingMessageSize: config.maxIncomingMessageSize,
134-
});
135-
136-
await wsHandler.onMessage(message);
137-
},
138-
onClose: async (_evt) => {
139-
await onOpenPromise;
140-
141-
logger().debug("websocket closed");
142-
143-
await wsHandler.onClose();
144-
},
145-
onError: async (error) => {
146-
await onOpenPromise;
147-
148-
// Actors don't need to know about this, since it's abstracted
149-
// away
150-
logger().warn("websocket error", { error: `${error}` });
151-
},
152-
};
103+
try {
104+
if (!handler.onConnectWebSocket)
105+
throw new Error("onConnectWebSocket is not implemented");
106+
107+
const encoding = getRequestEncoding(c.req);
108+
const parameters = getRequestConnectionParameters(c.req, config);
109+
110+
const wsHandler = await handler.onConnectWebSocket({
111+
req: c.req,
112+
encoding,
113+
parameters,
114+
});
115+
116+
const { promise: onOpenPromise, resolve: onOpenResolve } =
117+
Promise.withResolvers<undefined>();
118+
return {
119+
onOpen: async (_evt, ws) => {
120+
try {
121+
logger().debug("websocket open");
122+
123+
// Call handler
124+
await wsHandler.onOpen(ws);
125+
126+
// Resolve promise
127+
onOpenResolve(undefined);
128+
} catch (error) {
129+
const { code } = deconstructError(error, logger(), {
130+
wsEvent: "open",
131+
});
132+
ws.close(1011, code);
133+
}
134+
},
135+
onMessage: async (evt, ws) => {
136+
try {
137+
await onOpenPromise;
138+
139+
logger().debug("received message");
140+
141+
const value = evt.data.valueOf() as InputData;
142+
const message = await parseMessage(value, {
143+
encoding: encoding,
144+
maxIncomingMessageSize: config.maxIncomingMessageSize,
145+
});
146+
147+
await wsHandler.onMessage(message);
148+
} catch (error) {
149+
const { code } = deconstructError(error, logger(), {
150+
wsEvent: "message",
151+
});
152+
ws.close(1011, code);
153+
}
154+
},
155+
onClose: async (_evt) => {
156+
try {
157+
await onOpenPromise;
158+
159+
logger().debug("websocket closed");
160+
161+
await wsHandler.onClose();
162+
} catch (error) {
163+
deconstructError(error, logger(), { wsEvent: "close" });
164+
}
165+
},
166+
onError: async (error) => {
167+
try {
168+
await onOpenPromise;
169+
170+
// Actors don't need to know about this, since it's abstracted
171+
// away
172+
logger().warn("websocket error", { error: `${error}` });
173+
} catch (error) {
174+
deconstructError(error, logger(), { wsEvent: "error" });
175+
}
176+
},
177+
};
178+
} catch (error) {
179+
deconstructError(error, logger(), {});
180+
return {};
181+
}
153182
}),
154183
);
155184
} else {
@@ -233,42 +262,20 @@ export function createActorRouter(
233262
} satisfies protoHttpRpc.ResponseOk);
234263
} catch (error) {
235264
// Build response error information similar to WebSocket handling
236-
let status: ContentfulStatusCode;
237-
let code: string;
238-
let message: string;
239-
let metadata: unknown = undefined;
240-
241-
if (error instanceof errors.ActorError && error.public) {
242-
logger().info("http rpc public error", {
243-
rpc: rpcName,
244-
error,
245-
});
246265

247-
status = 400;
248-
code = error.code;
249-
message = String(error);
250-
metadata = error.metadata;
251-
} else {
252-
logger().warn("http rpc internal error", {
253-
rpc: rpcName,
254-
error,
255-
});
256-
257-
status = 500;
258-
code = errors.INTERNAL_ERROR_CODE;
259-
message = errors.INTERNAL_ERROR_DESCRIPTION;
260-
metadata = {
261-
//url: `https://hub.rivet.gg/projects/${this.#driver.metadata.project.slug}/environments/${this.#driver.metadata.environment.slug}/actors?actorId=${this.#driver.metadata.actor.id}`,
262-
} satisfies errors.InternalErrorMetadata;
263-
}
266+
const { statusCode, code, message, metadata } = deconstructError(
267+
error,
268+
logger(),
269+
{ rpc: rpcName },
270+
);
264271

265272
return c.json(
266273
{
267274
c: code,
268275
m: message,
269276
md: metadata,
270277
} satisfies protoHttpRpc.ResponseErr,
271-
{ status },
278+
{ status: statusCode },
272279
);
273280
}
274281
});
@@ -319,40 +326,19 @@ export function createActorRouter(
319326
return c.json({});
320327
} catch (error) {
321328
// Build response error information similar to WebSocket handling
322-
let status: ContentfulStatusCode;
323-
let code: string;
324-
let message: string;
325-
let metadata: unknown = undefined;
326-
327-
if (error instanceof errors.ActorError && error.public) {
328-
logger().info("http rpc public error", {
329-
error,
330-
});
331-
332-
status = 400;
333-
code = error.code;
334-
message = String(error);
335-
metadata = error.metadata;
336-
} else {
337-
logger().warn("http rpc internal error", {
338-
error,
339-
});
340-
341-
status = 500;
342-
code = errors.INTERNAL_ERROR_CODE;
343-
message = errors.INTERNAL_ERROR_DESCRIPTION;
344-
metadata = {
345-
//url: `https://hub.rivet.gg/projects/${this.#driver.metadata.project.slug}/environments/${this.#driver.metadata.environment.slug}/actors?actorId=${this.#driver.metadata.actor.id}`,
346-
} satisfies errors.InternalErrorMetadata;
347-
}
329+
const { statusCode, code, message, metadata } = deconstructError(
330+
error,
331+
logger(),
332+
{},
333+
);
348334

349335
return c.json(
350336
{
351337
c: code,
352338
m: message,
353339
md: metadata,
354340
} satisfies protoHttpRpc.ResponseErr,
355-
{ status },
341+
{ status: statusCode },
356342
);
357343
}
358344
});
@@ -362,9 +348,8 @@ export function createActorRouter(
362348
createInspectorRouter(handler.upgradeWebSocket, handler.onConnectInspector),
363349
);
364350

365-
app.notFound((c) => {
366-
return c.text("Not Found (ActorCore)", 404);
367-
});
351+
app.notFound(handleRouteNotFound);
352+
app.onError(handleRouteError);
368353

369354
return app;
370355
}

‎packages/actor-core/src/actor/runtime/inspect.ts

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import { throttle } from "@/actor/runtime/utils";
44
import type { UpgradeWebSocket, WSContext } from "hono/ws";
55
import { Hono, type HonoRequest } from "hono";
66
import * as errors from "@/actor/errors";
7-
import { safeStringify } from "@/common/utils";
7+
import { deconstructError, safeStringify } from "@/common/utils";
88
import {
99
type ToServer,
1010
ToServerSchema,
1111
} from "@/actor/protocol/inspector/to_server";
1212
import type { ToClient } from "@/actor/protocol/inspector/to_client";
13+
import { logger } from "./log";
1314

1415
export interface ConnectInspectorOpts {
1516
req: HonoRequest;
@@ -38,32 +39,55 @@ export function createInspectorRouter(
3839
if (!upgradeWebSocket || !onConnect) {
3940
return app.get("/", async (c) => {
4041
return c.json({
41-
error: "Inspector disabled. Only aviailable on WebSocket connections.",
42+
error: "Inspector disabled. Only available on WebSocket connections.",
4243
});
4344
});
4445
}
4546
return app.get(
4647
"/",
4748
upgradeWebSocket(async (c) => {
48-
const handler = await onConnect({ req: c.req });
49-
return {
50-
onOpen: async (_, ws) => {
51-
await handler.onOpen(ws);
52-
},
53-
onClose: async () => {
54-
await handler.onClose();
55-
},
56-
onMessage: async (event) => {
57-
try {
58-
const result = ToServerSchema.parse(
59-
JSON.parse(event.data.valueOf() as string),
60-
);
61-
await handler.onMessage(result);
62-
} catch (error) {
63-
throw new errors.MalformedMessage(error);
64-
}
65-
},
66-
};
49+
try {
50+
const handler = await onConnect({ req: c.req });
51+
return {
52+
onOpen: async (_, ws) => {
53+
try {
54+
await handler.onOpen(ws);
55+
} catch (error) {
56+
const { code } = deconstructError(error, logger(), {
57+
wsEvent: "open",
58+
});
59+
ws.close(1011, code);
60+
}
61+
},
62+
onClose: async () => {
63+
try {
64+
await handler.onClose();
65+
} catch (error) {
66+
deconstructError(error, logger(), {
67+
wsEvent: "close",
68+
});
69+
}
70+
},
71+
onMessage: async (event, ws) => {
72+
try {
73+
const { success, data, error } = ToServerSchema.safeParse(
74+
JSON.parse(event.data.valueOf() as string),
75+
);
76+
if (!success) throw new errors.MalformedMessage(error);
77+
78+
await handler.onMessage(data);
79+
} catch (error) {
80+
const { code } = deconstructError(error, logger(), {
81+
wsEvent: "message",
82+
});
83+
ws.close(1011, code);
84+
}
85+
},
86+
};
87+
} catch (error) {
88+
deconstructError(error, logger(), {});
89+
return {};
90+
}
6791
}),
6892
);
6993
}

0 commit comments

Comments
 (0)