From e05bb4b84a0414da4bdde2e4361c186903693e6d Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Thu, 10 Sep 2026 12:35:51 +0000 Subject: [PATCH] Normalise non-Error values before reporting to Sentry The simulator's internal_error message carries Emscripten's ExitStatus, a plain constructor that does not extend Error. After structured cloning over postMessage it is a bare {name, message, status} object, which Sentry titles "Object captured as exception with keys: ..." and groups together regardless of cause. reportError now converts any non-Error into an Error carrying the original name and message, with remaining fields merged into extra alongside any caller-supplied context. The synthesised stack is dropped because it would only show the logging call chain, identical for every caller, and Sentry would group unrelated errors together on it. Fixes #1319 --- src/logging/sentry.test.ts | 90 ++++++++++++++++++++++++++++++++++++++ src/logging/sentry.ts | 46 ++++++++++++++++++- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/logging/sentry.test.ts diff --git a/src/logging/sentry.test.ts b/src/logging/sentry.test.ts new file mode 100644 index 000000000..50425e1b9 --- /dev/null +++ b/src/logging/sentry.test.ts @@ -0,0 +1,90 @@ +/** + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { captureException } from "@sentry/browser"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { reportError } from "./sentry"; + +vi.mock("@sentry/browser", () => ({ + addBreadcrumb: vi.fn(), + captureException: vi.fn(), + init: vi.fn(), +})); + +const dsn = "https://example@sentry.example.com/1"; + +const captured = () => { + expect(captureException).toHaveBeenCalledTimes(1); + return vi.mocked(captureException).mock.calls[0]; +}; + +describe("reportError", () => { + beforeEach(() => { + vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + it("does nothing beyond console when Sentry is disabled", () => { + reportError(undefined, "Oops", new Error("boom")); + expect(captureException).not.toHaveBeenCalled(); + }); + + it("passes Error instances through unchanged", () => { + const e = new Error("boom"); + reportError(dsn, "Oops", e); + const [error, hint] = captured(); + expect(error).toBe(e); + expect(hint).toBeUndefined(); + }); + + it("converts Error-like objects, keeping other fields as extra", () => { + // Shape of Emscripten's ExitStatus after structured clone over postMessage. + reportError(dsn, "Simulator internal error", { + name: "ExitStatus", + message: "Program terminated with exit(1)", + status: 1, + }); + const [error, hint] = captured(); + expect(error).toBeInstanceOf(Error); + expect((error as Error).name).toBe("ExitStatus"); + expect((error as Error).message).toBe("Program terminated with exit(1)"); + expect((error as Error).stack).toBeUndefined(); + expect(hint).toEqual({ extra: { status: 1 } }); + }); + + it("merges caller context with fields from the object", () => { + reportError( + dsn, + "Oops", + { message: "boom", status: 1, phase: "object" }, + { phase: "caller", docLength: 3 } + ); + const [, hint] = captured(); + expect(hint).toEqual({ + extra: { status: 1, phase: "caller", docLength: 3 }, + }); + }); + + it("attaches caller context to Error instances", () => { + reportError(dsn, "Oops", new Error("boom"), { phase: "update" }); + const [, hint] = captured(); + expect(hint).toEqual({ extra: { phase: "update" } }); + }); + + it("serialises objects without a message", () => { + reportError(dsn, "Oops", { code: 42 }); + const [error, hint] = captured(); + expect((error as Error).name).toBe("Error"); + expect((error as Error).message).toBe('{"code":42}'); + expect(hint).toEqual({ extra: { code: 42 } }); + }); + + it("converts primitives", () => { + reportError(dsn, "Oops", "just a string"); + const [error, hint] = captured(); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("just a string"); + expect(hint).toBeUndefined(); + }); +}); diff --git a/src/logging/sentry.ts b/src/logging/sentry.ts index 7eac0a08a..0f14210b2 100644 --- a/src/logging/sentry.ts +++ b/src/logging/sentry.ts @@ -40,6 +40,12 @@ export const initSentry = (env: Record): string | undefined => { /** * Report an error to Sentry (if configured) and the console. + * + * Non-Error values are converted before capture. Sentry otherwise titles + * them "Object captured as exception with keys: ..." and groups them all + * together, which is what happens to errors structured-cloned across + * postMessage from the simulator (e.g. Emscripten's ExitStatus, which + * does not extend Error). */ export const reportError = ( dsn: string | undefined, @@ -61,12 +67,50 @@ export const reportError = ( type: "error-message", level: "error", }); - sentryCaptureException(e, context ? { extra: context } : undefined); + const { error, extra } = toError(e); + const combined = extra || context ? { ...extra, ...context } : undefined; + sentryCaptureException(error, combined ? { extra: combined } : undefined); } catch (err) { console.error(err); } }; +interface NormalisedError { + error: Error; + extra?: Record; +} + +const toError = (e: unknown): NormalisedError => { + if (e instanceof Error) { + return { error: e }; + } + let error: Error; + let extra: Record | undefined; + if (typeof e === "object" && e !== null) { + const { name, message, ...rest } = e as Record; + error = new Error(typeof message === "string" ? message : stringify(e)); + if (typeof name === "string" && name) { + error.name = name; + } + extra = Object.keys(rest).length > 0 ? rest : undefined; + } else { + error = new Error(String(e)); + } + // The stack we'd get here is just the logging call chain, identical for + // every caller, so Sentry would group unrelated errors together. Without + // one it falls back to grouping by type and message. + error.stack = undefined; + return { error, extra }; +}; + +const stringify = (e: object): string => { + try { + return JSON.stringify(e); + } catch { + return Object.prototype.toString.call(e); + } +}; + /** * Add a breadcrumb to Sentry, or console-log it as a fallback when * Sentry isn't configured. Used to record analytics events as context