From 949f77c5f5e63779d8c0d527e1ea163c68975552 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 14 Sep 2023 16:41:53 +0200 Subject: [PATCH 1/2] feat(node): Improve non-error messages --- .../suites/public-api/captureException/empty-obj/test.ts | 2 +- .../remix/test/integration/test/server/action.test.ts | 8 ++++---- packages/utils/src/eventbuilder.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/node-integration-tests/suites/public-api/captureException/empty-obj/test.ts b/packages/node-integration-tests/suites/public-api/captureException/empty-obj/test.ts index b30b64c426fd..37d5ed8b9855 100644 --- a/packages/node-integration-tests/suites/public-api/captureException/empty-obj/test.ts +++ b/packages/node-integration-tests/suites/public-api/captureException/empty-obj/test.ts @@ -9,7 +9,7 @@ test('should capture an empty object', async () => { values: [ { type: 'Error', - value: 'Non-Error exception captured with keys: [object has no keys]', + value: 'Object captured as exception with keys: [object has no keys]', mechanism: { type: 'generic', handled: true, diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts index af48c99777ce..fdeb70962a3e 100644 --- a/packages/remix/test/integration/test/server/action.test.ts +++ b/packages/remix/test/integration/test/server/action.test.ts @@ -304,8 +304,8 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada { type: 'Error', value: useV2 - ? 'Non-Error exception captured with keys: data, internal, status, statusText' - : 'Non-Error exception captured with keys: data', + ? 'Object captured as exception with keys: data, internal, status, statusText' + : 'Object captured as exception with keys: data', stacktrace: expect.any(Object), mechanism: { data: { @@ -412,8 +412,8 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada { type: 'Error', value: useV2 - ? 'Non-Error exception captured with keys: data, internal, status, statusText' - : 'Non-Error exception captured with keys: [object has no keys]', + ? 'Object captured as exception with keys: data, internal, status, statusText' + : 'Object captured as exception with keys: [object has no keys]', stacktrace: expect.any(Object), mechanism: { data: { diff --git a/packages/utils/src/eventbuilder.ts b/packages/utils/src/eventbuilder.ts index 01e217921d87..bd2565246758 100644 --- a/packages/utils/src/eventbuilder.ts +++ b/packages/utils/src/eventbuilder.ts @@ -61,7 +61,7 @@ export function eventFromUnknownInput( if (isPlainObject(exception)) { // This will allow us to group events based on top-level keys // which is much better than creating new group when any key/value change - const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`; + const message = `Object captured as exception with keys: ${extractExceptionKeysForMessage(exception)}`; const hub = getCurrentHub(); const client = hub.getClient(); From bdcc38c23aa1b0cca8dd46a93c384d92ad4a4119 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 14 Sep 2023 17:28:55 +0200 Subject: [PATCH 2/2] Handle objects with name and message properties --- packages/utils/src/eventbuilder.ts | 25 +++++++++++++++--- packages/utils/test/eventbuilder.test.ts | 32 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 packages/utils/test/eventbuilder.test.ts diff --git a/packages/utils/src/eventbuilder.ts b/packages/utils/src/eventbuilder.ts index bd2565246758..03af0b3d1905 100644 --- a/packages/utils/src/eventbuilder.ts +++ b/packages/utils/src/eventbuilder.ts @@ -39,6 +39,26 @@ export function exceptionFromError(stackParser: StackParser, error: Error): Exce return exception; } +function getMessageForObject(exception: object): string { + if ('name' in exception && typeof exception.name === 'string') { + let message = `'${exception.name}' captured as exception`; + + if ('message' in exception && typeof exception.message === 'string') { + message += ` with message '${exception.message}'`; + } + + return message; + } else if ('message' in exception && typeof exception.message === 'string') { + return exception.message; + } else { + // This will allow us to group events based on top-level keys + // which is much better than creating new group when any key/value change + return `Object captured as exception with keys: ${extractExceptionKeysForMessage( + exception as Record, + )}`; + } +} + /** * Builds and Event from a Exception * @hidden @@ -59,10 +79,6 @@ export function eventFromUnknownInput( if (!isError(exception)) { if (isPlainObject(exception)) { - // This will allow us to group events based on top-level keys - // which is much better than creating new group when any key/value change - const message = `Object captured as exception with keys: ${extractExceptionKeysForMessage(exception)}`; - const hub = getCurrentHub(); const client = hub.getClient(); const normalizeDepth = client && client.getOptions().normalizeDepth; @@ -70,6 +86,7 @@ export function eventFromUnknownInput( scope.setExtra('__serialized__', normalizeToSize(exception, normalizeDepth)); }); + const message = getMessageForObject(exception); ex = (hint && hint.syntheticException) || new Error(message); (ex as Error).message = message; } else { diff --git a/packages/utils/test/eventbuilder.test.ts b/packages/utils/test/eventbuilder.test.ts new file mode 100644 index 000000000000..137860b16ce4 --- /dev/null +++ b/packages/utils/test/eventbuilder.test.ts @@ -0,0 +1,32 @@ +import type { Hub } from '@sentry/types'; + +import { createStackParser, eventFromUnknownInput, nodeStackLineParser } from '../src'; + +function getCurrentHub(): Hub { + // Some fake hub to get us through + return { getClient: () => undefined, configureScope: () => {} } as unknown as Hub; +} + +const stackParser = createStackParser(nodeStackLineParser()); + +describe('eventFromUnknownInput', () => { + test('object with useless props', () => { + const event = eventFromUnknownInput(getCurrentHub, stackParser, { foo: { bar: 'baz' }, prop: 1 }); + expect(event.exception?.values?.[0].value).toBe('Object captured as exception with keys: foo, prop'); + }); + + test('object with name prop', () => { + const event = eventFromUnknownInput(getCurrentHub, stackParser, { foo: { bar: 'baz' }, name: 'BadType' }); + expect(event.exception?.values?.[0].value).toBe("'BadType' captured as exception"); + }); + + test('object with name and message props', () => { + const event = eventFromUnknownInput(getCurrentHub, stackParser, { message: 'went wrong', name: 'BadType' }); + expect(event.exception?.values?.[0].value).toBe("'BadType' captured as exception with message 'went wrong'"); + }); + + test('object with message prop', () => { + const event = eventFromUnknownInput(getCurrentHub, stackParser, { foo: { bar: 'baz' }, message: 'Some message' }); + expect(event.exception?.values?.[0].value).toBe('Some message'); + }); +});