Fix: Router-worker loss of sentry + metrics on error

packages/workers-shared/asset-worker/src/handler.ts

@@ -5,7 +5,7 @@ import {
 	NotModifiedResponse,
 	OkResponse,
 	TemporaryRedirectResponse,
-} from "./responses";
+} from "../../utils/responses";
 import { getHeaders } from "./utils/headers";
 import type { AssetConfig } from "../../utils/types";
 import type EntrypointType from "./index";

packages/workers-shared/asset-worker/src/index.ts

@@ -1,12 +1,12 @@
 import { WorkerEntrypoint } from "cloudflare:workers";
 import { PerformanceTimer } from "../../utils/performance";
+import { InternalServerErrorResponse } from "../../utils/responses";
 import { setupSentry } from "../../utils/sentry";
 import { mockJaegerBinding } from "../../utils/tracing";
 import { Analytics } from "./analytics";
 import { AssetsManifest } from "./assets-manifest";
 import { applyConfigurationDefaults } from "./configuration";
 import { decodePath, getIntent, handleRequest } from "./handler";
-import { InternalServerErrorResponse } from "./responses";
 import { getAssetWithMetadataFromKV } from "./utils/kv";
 import type {
 	AssetWorkerConfig,

packages/workers-shared/router-worker/src/index.ts

@@ -1,4 +1,5 @@
 import { PerformanceTimer } from "../../utils/performance";
+import { InternalServerErrorResponse } from "../../utils/responses";
 import { setupSentry } from "../../utils/sentry";
 import { mockJaegerBinding } from "../../utils/tracing";
 import { Analytics, DISPATCH_TYPE } from "./analytics";
@@ -9,6 +10,7 @@ import type {
 	UnsafePerformanceTimer,
 } from "../../utils/types";
 import type { ColoMetadata, Environment, ReadyAnalytics } from "./types";
+import type { Toucan } from "toucan-js";
 
 interface Env {
 	ASSET_WORKER: Service<AssetWorker>;
@@ -80,7 +82,9 @@ export default {
 						);
 					}
 					return env.USER_WORKER.fetch(maybeSecondRequest);
-				});
+				})
+					.catch((err) => handleError(sentry, analytics, err))
+					.finally(() => submitMetrics(analytics, performance, startTimeMs));
 			}
 
 			// Otherwise, we try to first fetch assets, falling back to user-Worker.
@@ -103,7 +107,9 @@ export default {
 						analytics.setData({ dispatchtype: DISPATCH_TYPE.WORKER });
 						return env.USER_WORKER.fetch(maybeSecondRequest);
 					}
-				});
+				})
+					.catch((err) => handleError(sentry, analytics, err))
+					.finally(() => submitMetrics(analytics, performance, startTimeMs));
 			}
 
 			return env.JAEGER.enterSpan("assets_only", async (span) => {
@@ -114,7 +120,9 @@ export default {
 
 				analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
 				return env.ASSET_WORKER.fetch(request);
-			});
+			})
+				.catch((err) => handleError(sentry, analytics, err))
+				.finally(() => submitMetrics(analytics, performance, startTimeMs));
 		} catch (err) {
 			if (err instanceof Error) {
 				analytics.setData({ error: err.message });
@@ -131,3 +139,42 @@ export default {
 		}
 	},
 };
+
+// TODO: it would be nice to share this with asset-worker, but we'd have to refactor
+// Analytics, as it differs between AW and RW. For now, leaving in the code duplication.
+function handleError(
+	sentry: Toucan | undefined,
+	analytics: Analytics,
+	err: unknown
+) {
+	try {
+		const response = new InternalServerErrorResponse(err as Error);
+
+		// Log to Sentry if we can
+		if (sentry) {
+			sentry.captureException(err);
+		}
+
+		if (err instanceof Error) {
+			analytics.setData({ error: err.message });
+		}
+
+		return response;
+	} catch (e) {
+		console.error("Error handling error", e);
+		return new InternalServerErrorResponse(e as Error);
+	}
+}
+
+function submitMetrics(
+	analytics: Analytics,
+	performance: PerformanceTimer,
+	startTimeMs: number
+) {
+	try {
+		analytics.setData({ requestTime: performance.now() - startTimeMs });
+		analytics.write();
+	} catch (e) {
+		console.error("Error submitting metrics", e);
+	}
+}