From bcba32cf64e8cab0a5edb8e687c084747b96b29b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 14 Feb 2020 12:08:32 +0100 Subject: [PATCH 1/3] add tests --- .../suites/onunhandledrejection.js | 93 +++++++++++++++++-- 1 file changed, 86 insertions(+), 7 deletions(-) diff --git a/packages/browser/test/integration/suites/onunhandledrejection.js b/packages/browser/test/integration/suites/onunhandledrejection.js index b1e71e987376..eab76033b5d1 100644 --- a/packages/browser/test/integration/suites/onunhandledrejection.js +++ b/packages/browser/test/integration/suites/onunhandledrejection.js @@ -30,6 +30,85 @@ describe("window.onunhandledrejection", function() { }); }); + // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents + // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into + // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec + // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and + // https://github.com/getsentry/sentry-javascript/issues/2380 + it("should capture PromiseRejectionEvent cast to CustomEvent with type unhandledrejection", function() { + return runInSandbox(sandbox, function() { + if (supportsOnunhandledRejection()) { + // this isn't how it happens in real life, in that the promise and reason + // values come from an actual PromiseRejectionEvent, but it's enough to test + // how the SDK handles the structure + window.dispatchEvent( + new CustomEvent("unhandledrejection", { + detail: { + promise: new Promise(() => {}), + // we're testing with an error here but it could be anything - really + // all we're testing is that it gets dug out correctly + reason: new Error("test2"), + }, + }) + ); + Promise.reject(); + } else { + window.resolveTest({ window: window }); + } + }).then(function(summary) { + if (summary.window.supportsOnunhandledRejection()) { + assert.equal(summary.events[0].exception.values[0].value, "test2"); + assert.equal(summary.events[0].exception.values[0].type, "Error"); + + // Of course Safari had to screw up here... + if (!/Version\/\d.+Safari\/\d/.test(window.navigator.userAgent)) { + assert.isAtLeast( + summary.events[0].exception.values[0].stacktrace.frames.length, + 1 + ); + } + assert.equal( + summary.events[0].exception.values[0].mechanism.handled, + false + ); + assert.equal( + summary.events[0].exception.values[0].mechanism.type, + "onunhandledrejection" + ); + } + }); + }); + + // there's no evidence that this actually happens, but it could, and our code correctly + // handles it, so might as well prevent future regression on that score + it("should capture a random Event with type unhandledrejection", function() { + return runInSandbox(sandbox, function() { + if (supportsOnunhandledRejection()) { + window.dispatchEvent(new Event("unhandledrejection")); + Promise.reject(); + } else { + window.resolveTest({ window: window }); + } + }).then(function(summary) { + if (summary.window.supportsOnunhandledRejection()) { + // non-error rejections don't provide stacktraces so we can skip that assertion + assert.equal( + summary.events[0].exception.values[0].value, + "Non-Error promise rejection captured with keys: currentTarget, isTrusted, target, type" + ); + assert.equal(summary.events[0].exception.values[0].type, "Event"); + assert.equal( + summary.events[0].exception.values[0].mechanism.handled, + false + ); + assert.equal( + summary.events[0].exception.values[0].mechanism.type, + "onunhandledrejection" + ); + } + }); + }); + it("should capture unhandledrejection with a string", function() { return runInSandbox(sandbox, function() { if (supportsOnunhandledRejection()) { @@ -39,7 +118,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with value: test" @@ -69,7 +148,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal(summary.events[0].exception.values[0].value.length, 253); assert.include( summary.events[0].exception.values[0].value, @@ -100,7 +179,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with keys: a, b, c" @@ -137,7 +216,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with keys: a, b, c, d, e" @@ -167,7 +246,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with value: 1337" @@ -197,7 +276,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with value: null" @@ -227,7 +306,7 @@ describe("window.onunhandledrejection", function() { } }).then(function(summary) { if (summary.window.supportsOnunhandledRejection()) { - // non-error rejections doesnt provide stacktraces so we can skip the assertion + // non-error rejections don't provide stacktraces so we can skip that assertion assert.equal( summary.events[0].exception.values[0].value, "Non-Error promise rejection captured with value: undefined" From 494fa66441b7e1c6cbc6e716d9ab15d452bf64a0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 14 Feb 2020 17:52:03 +0100 Subject: [PATCH 2/3] remove stray promise rejection --- .../test/integration/suites/onunhandledrejection.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/browser/test/integration/suites/onunhandledrejection.js b/packages/browser/test/integration/suites/onunhandledrejection.js index eab76033b5d1..47528014c546 100644 --- a/packages/browser/test/integration/suites/onunhandledrejection.js +++ b/packages/browser/test/integration/suites/onunhandledrejection.js @@ -75,6 +75,9 @@ describe("window.onunhandledrejection", function() { summary.events[0].exception.values[0].mechanism.type, "onunhandledrejection" ); + // even though it's a regular Event (rather than a PRE) it should sill only + // come through this channel + assert.equal(summary.events.length, 1); } }); }); @@ -85,7 +88,6 @@ describe("window.onunhandledrejection", function() { return runInSandbox(sandbox, function() { if (supportsOnunhandledRejection()) { window.dispatchEvent(new Event("unhandledrejection")); - Promise.reject(); } else { window.resolveTest({ window: window }); } @@ -105,6 +107,9 @@ describe("window.onunhandledrejection", function() { summary.events[0].exception.values[0].mechanism.type, "onunhandledrejection" ); + // even though it's a regular Event (rather than a PRE) it should sill only + // come through this channel + assert.equal(summary.events.length, 1); } }); }); From 4e7ab65125b1cf632bfd474b8d972863075744d2 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 17 Feb 2020 10:30:06 +0100 Subject: [PATCH 3/3] fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Kamil Ogórek --- .../browser/test/integration/suites/onunhandledrejection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/test/integration/suites/onunhandledrejection.js b/packages/browser/test/integration/suites/onunhandledrejection.js index 47528014c546..0d47c47347b4 100644 --- a/packages/browser/test/integration/suites/onunhandledrejection.js +++ b/packages/browser/test/integration/suites/onunhandledrejection.js @@ -75,7 +75,7 @@ describe("window.onunhandledrejection", function() { summary.events[0].exception.values[0].mechanism.type, "onunhandledrejection" ); - // even though it's a regular Event (rather than a PRE) it should sill only + // even though it's a regular Event (rather than a PRE) it should still only // come through this channel assert.equal(summary.events.length, 1); }