Skip to content

Commit

Permalink
feat(stdlib): make AppError compatible with Sentry
Browse files Browse the repository at this point in the history
By setting the `Error#message`, users don't need separate handling of AppErrors when capturing them with Sentry. In combination with the extra error data integration, the `info` prop is captured as well.
  • Loading branch information
dirkdev98 committed Apr 26, 2024
1 parent 2134362 commit 056d402
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 15 deletions.
27 changes: 13 additions & 14 deletions packages/server/src/middleware/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,17 @@ export function errorHandler(opts) {
err = AppError.serverError({}, err);
}

if (err.status >= 500) {
if (_compasSentryExport) {
if (err === origErr) {
// An AppError.serverError was thrown.
_compasSentryExport.captureException(
new Error(err.key, {
cause: err,
}),
);
} else {
// Something else was thrown.
_compasSentryExport.captureException(origErr);
}
}
const isUnexpectedError = err.status >= 500;

if (isUnexpectedError) {
// Upgrade to error logging, developer probably has alerting on error logs.
log = ctx.log.error;
}

if (_compasSentryExport && isUnexpectedError) {
_compasSentryExport.captureException(origErr);
}

ctx.status = err.status;

const formatted = AppError.format(err);
Expand All @@ -66,8 +59,14 @@ export function errorHandler(opts) {

if (onAppError === defaultOnAppError) {
if (isProduction()) {
// Delete generic internals
delete formatted.stack;
delete formatted.cause;

if (isUnexpectedError) {
// Remove any possible internal details. In explicit 400's for example, info is useful to the caller.
delete formatted.info;
}
}

ctx.body = formatted;
Expand Down
11 changes: 10 additions & 1 deletion packages/stdlib/src/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,16 @@ export class AppError extends Error {
* @param {Error} [cause]
*/
constructor(key, status, info, cause) {
super();
let errMessage = info?.message ?? info?.type ?? "";
if (typeof errMessage !== "string") {
errMessage = "";
}

if (errMessage) {
errMessage = `: ${errMessage}`;
}

super(`AppError: ${key}${errMessage}`);

this.key = key;
this.status = status;
Expand Down
26 changes: 26 additions & 0 deletions packages/stdlib/src/error.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ test("stdlib/error", (t) => {
const e = new AppError(500, 500);

t.equal(AppError.instanceOf(e), true);
t.equal(e.message, "AppError: error.server.internal");
t.equal(e.key, "error.server.internal");
t.equal(e.info.appErrorConstructParams.key, 500);
t.equal(e.cause.key, 500);
Expand All @@ -20,18 +21,43 @@ test("stdlib/error", (t) => {
const e = new AppError("test.error", "500");

t.equal(AppError.instanceOf(e), true);
t.equal(e.message, "AppError: error.server.internal");
t.equal(e.key, "error.server.internal");
t.equal(e.info.appErrorConstructParams.key, "test.error");
t.equal(e.info.appErrorConstructParams.status, "500");
t.equal(e.cause.key, "test.error");
t.equal(e.cause.status, "500");
});

t.test("AppError sets Error#message based on info object", (t) => {
t.equal(new AppError("foo", 200, {}).message, "AppError: foo");
t.equal(
new AppError("foo", 200, {
message: "message prop",
}).message,
"AppError: foo: message prop",
);
t.equal(
new AppError("foo", 200, {
type: "type prop",
}).message,
"AppError: foo: type prop",
);
t.equal(
new AppError("foo", 200, {
type: "type prop",
message: "prefers message",
}).message,
"AppError: foo: prefers message",
);
});

t.test("AppError#format with stack", (t) => {
try {
throw AppError.validationError("test.error", {});
} catch (e) {
t.ok(AppError.instanceOf(e));
t.equal(e.message, "AppError: test.error");

const formatted = AppError.format(e);
t.equal(formatted.key, "test.error");
Expand Down

0 comments on commit 056d402

Please sign in to comment.