From e7c770709b79fb2ef2c8ebe582d8987dbc1363c5 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 17 Jul 2021 14:41:45 +0300 Subject: [PATCH] fix(docs-infra): correctly handle reported errors (#42883) Error-handling in AIO happens mainly in two places: 1. For errors happening inside the app we have a custom `ErrorHandler` implementation, `ReportingErrorHandler`. `ReportingErrorHandler` passes errors to the default `ErrorHandler` (for them to be logged to the console) and also forwards them to `window.onerror()`. 2. Errors happening outside the app and errors forwarded by `ReportingErrorHandler` are handled by `window.onerror()`, which in turn reports them to Google analytics. Previously, we were making some assumptions (which turned out to be incorrect based on the info captured in Google analytics - see #28106): - `ReportingErrorHandler` assumed that the errors passed to its `handleError()` method would be either strings or `Error` instances. _Apparently, other values (such as `null` or `undefined`) may also be passed._ - `window.onerror()` assumed that if an `Error` instance was passed in, it would always have a stacktrace (i.e. its `stack` property would be defined). _This is not necessarily true, although it is not clear (based on the logs) whether reported errors of this type are caused by `Error` instance with no stacktrace or by non-string error objects which are incorrectly treated as `Error` instances. This commit ensures that all types of error arguments can be handled correctly, including `Error` instances with no stacktrace and other types of objects or primitives. NOTE: PR #42881 is related as it fixes handling `null` and `undefined` arguments in the default `ErrorHandler`. Fixes #28106 PR Close #42883 --- .../shared/reporting-error-handler.spec.ts | 30 +++++++++++++++++++ aio/src/app/shared/reporting-error-handler.ts | 18 +++++++---- aio/src/index.html | 2 +- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/aio/src/app/shared/reporting-error-handler.spec.ts b/aio/src/app/shared/reporting-error-handler.spec.ts index 39fb790e30..941327b22f 100644 --- a/aio/src/app/shared/reporting-error-handler.spec.ts +++ b/aio/src/app/shared/reporting-error-handler.spec.ts @@ -56,10 +56,40 @@ describe('ReportingErrorHandler service', () => { expect(onerrorSpy).toHaveBeenCalledWith(error.message, undefined, undefined, undefined, error); }); + it('should send a non-error object to window.onerror', () => { + const error = {reason: 'this is an error message'}; + handler.handleError(error); + expect(onerrorSpy).toHaveBeenCalledWith(JSON.stringify(error)); + }); + + it('should send a non-error object with circular references to window.onerror', () => { + const error = { + reason: 'this is an error message', + get self() { return this; }, + toString() { return `{reason: ${this.reason}}`; }, + }; + handler.handleError(error); + expect(onerrorSpy).toHaveBeenCalledWith('{reason: this is an error message}'); + }); + it('should send an error string to window.onerror', () => { const error = 'this is an error message'; handler.handleError(error); expect(onerrorSpy).toHaveBeenCalledWith(error); }); + + it('should send a non-object, non-string error stringified to window.onerror', () => { + handler.handleError(404); + handler.handleError(false); + handler.handleError(null); + handler.handleError(undefined); + + expect(onerrorSpy.calls.allArgs()).toEqual([ + ['404'], + ['false'], + ['null'], + ['undefined'], + ]); + }); }); }); diff --git a/aio/src/app/shared/reporting-error-handler.ts b/aio/src/app/shared/reporting-error-handler.ts index 126580f157..badb88bdff 100644 --- a/aio/src/app/shared/reporting-error-handler.ts +++ b/aio/src/app/shared/reporting-error-handler.ts @@ -17,8 +17,7 @@ export class ReportingErrorHandler extends ErrorHandler { * Send error info to Google Analytics, in addition to the default handling. * @param error Information about the error. */ - handleError(error: string | Error) { - + handleError(error: any) { try { super.handleError(error); } catch (e) { @@ -27,12 +26,19 @@ export class ReportingErrorHandler extends ErrorHandler { this.reportError(error); } - private reportError(error: string | Error) { + private reportError(error: unknown) { if (this.window.onerror) { - if (typeof error === 'string') { - this.window.onerror(error); - } else { + if (error instanceof Error) { this.window.onerror(error.message, undefined, undefined, undefined, error); + } else { + if (typeof error === 'object') { + try { + error = JSON.stringify(error); + } catch { + // Ignore the error and just let it be stringified. + } + } + this.window.onerror(`${error}`); } } } diff --git a/aio/src/index.html b/aio/src/index.html index 2444bf4735..db945765a5 100644 --- a/aio/src/index.html +++ b/aio/src/index.html @@ -69,7 +69,7 @@ function formatError(msg, url, line, col, e) { var stack; msg = msg.replace(/^Error: /, ''); - if (e) { + if (e && e.stack) { stack = e.stack // strip the leading "Error: " from the stack trace .replace(/^Error: /, '')