From 3ab9c89762bb4fa724b56ed3569716b94a996c95 Mon Sep 17 00:00:00 2001 From: LarrMarburger Date: Thu, 10 Aug 2023 19:49:08 +0200 Subject: [PATCH] Fix failing tests due to failed error logging Unit and integration tests have been failing due to failed logging of `Error` objects. These were creating an issue where `mocha` was not properly returning right exit codes, leading to test pipelines incorrectly passing despite test failures. - Fix runtime behavior of failing to retrieve error stacks. - Add tests for error handling. - Add more robust custom error handling. Related issues: babel/babel#14273, vuejs/vue-cli#6994. --- src/application/Common/CustomError.ts | 50 +++++ .../Parser/NodeValidation/NodeDataError.ts | 5 +- .../application/Common/CustomError.spec.ts | 174 ++++++++++++++++++ .../NodeValidation/NodeDataError.spec.ts | 12 +- vue.config.js | 3 + 5 files changed, 232 insertions(+), 12 deletions(-) create mode 100644 src/application/Common/CustomError.ts create mode 100644 tests/unit/application/Common/CustomError.spec.ts diff --git a/src/application/Common/CustomError.ts b/src/application/Common/CustomError.ts new file mode 100644 index 0000000..f572cd0 --- /dev/null +++ b/src/application/Common/CustomError.ts @@ -0,0 +1,50 @@ +/* + Provides a unified and resilient way to extend errors across platforms. + + Rationale: + - Babel: + > "Built-in classes cannot be properly subclassed due to limitations in ES5" + > https://web.archive.org/web/20230810014108/https://babeljs.io/docs/caveats#classes + - TypeScript: + > "Extending built-ins like Error, Array, and Map may no longer work" + > https://web.archive.org/web/20230810014143/https://github.com/Microsoft/TypeScript-wiki/blob/main/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work +*/ +export abstract class CustomError extends Error { + constructor(message?: string, options?: ErrorOptions) { + super(message, options); + + fixPrototype(this, new.target.prototype); + ensureStackTrace(this); + + this.name = this.constructor.name; + } +} + +export const Environment = { + getSetPrototypeOf: () => Object.setPrototypeOf, + getCaptureStackTrace: () => Error.captureStackTrace, +}; + +function fixPrototype(target: Error, prototype: CustomError) { + // https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget + const setPrototypeOf = Environment.getSetPrototypeOf(); + if (!functionExists(setPrototypeOf)) { + return; + } + setPrototypeOf(target, prototype); +} + +function ensureStackTrace(target: Error) { + const captureStackTrace = Environment.getCaptureStackTrace(); + if (!functionExists(captureStackTrace)) { + // captureStackTrace is only available on V8, if it's not available + // modern JS engines will usually generate a stack trace on error objects when they're thrown. + return; + } + captureStackTrace(target, target.constructor); +} + +function functionExists(func: unknown): boolean { + // Not doing truthy/falsy check i.e. if(func) as most values are truthy in JS for robustness + return typeof func === 'function'; +} diff --git a/src/application/Parser/NodeValidation/NodeDataError.ts b/src/application/Parser/NodeValidation/NodeDataError.ts index 0324250..21c3e37 100644 --- a/src/application/Parser/NodeValidation/NodeDataError.ts +++ b/src/application/Parser/NodeValidation/NodeDataError.ts @@ -1,11 +1,10 @@ +import { CustomError } from '@/application/Common/CustomError'; import { NodeType } from './NodeType'; import { NodeData } from './NodeData'; -export class NodeDataError extends Error { +export class NodeDataError extends CustomError { constructor(message: string, public readonly context: INodeDataErrorContext) { super(createMessage(message, context)); - Object.setPrototypeOf(this, new.target.prototype); // https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-2.html#support-for-newtarget - this.name = new.target.name; } } diff --git a/tests/unit/application/Common/CustomError.spec.ts b/tests/unit/application/Common/CustomError.spec.ts new file mode 100644 index 0000000..8e736d7 --- /dev/null +++ b/tests/unit/application/Common/CustomError.spec.ts @@ -0,0 +1,174 @@ +// eslint-disable-next-line max-classes-per-file +import 'mocha'; +import { expect } from 'chai'; +import { CustomError, Environment } from '@/application/Common/CustomError'; + +describe('CustomError', () => { + afterEach(() => { + Environment.getSetPrototypeOf = () => Object.setPrototypeOf; + Environment.getCaptureStackTrace = () => Error.captureStackTrace; + }); + describe('sets members as expected', () => { + it('`name`', () => { + // arrange + const expectedName = CustomErrorConcrete.name; + // act + const sut = new CustomErrorConcrete(); + // assert + expect(sut.name).to.equal(expectedName); + }); + it('`message`', () => { + // arrange + const expectedMessage = 'expected message'; + // act + const sut = new CustomErrorConcrete(expectedMessage); + // assert + expect(sut.message).to.equal(expectedMessage); + }); + it('`cause`', () => { + // arrange + const expectedCause = new Error('expected cause'); + // act + const sut = new CustomErrorConcrete(undefined, { + cause: expectedCause, + }); + // assert + expect(sut.cause).to.equal(expectedCause); + }); + describe('`stack`', () => { + it('sets using `getCaptureStackTrace` if available', () => { + // arrange + const mockStackTrace = 'mocked stack trace'; + Environment.getCaptureStackTrace = () => (error) => { + (error as Error).stack = mockStackTrace; + }; + // act + const sut = new CustomErrorConcrete(); + // assert + expect(sut.stack).to.equal(mockStackTrace); + }); + it('defined', () => { + // arrange + const customError = new CustomErrorConcrete(); + + // act + const { stack } = customError; + + // assert + expect(stack).to.not.equal(undefined); + }); + }); + }); + describe('retains correct prototypes', () => { + it('instance of `Error`', () => { + // arrange + const expected = Error; + // act + const sut = new CustomErrorConcrete(); + // assert + expect(sut).to.be.an.instanceof(expected); + }); + it('instance of `CustomErrorConcrete`', () => { + // arrange + const expected = CustomErrorConcrete; + // act + const sut = new CustomErrorConcrete(); + // assert + expect(sut).to.be.an.instanceof(expected); + }); + it('instance of `CustomError`', () => { + // arrange + const expected = CustomError; + // act + const sut = new CustomErrorConcrete(); + // assert + expect(sut).to.be.an.instanceof(expected); + }); + it('thrown error retains `CustomError` type', () => { + // arrange + const expected = CustomError; + let thrownError: unknown; + // act + try { + throw new CustomErrorConcrete('message'); + } catch (e) { + thrownError = e; + } + // assert + expect(thrownError).to.be.an.instanceof(expected); + }); + }); + describe('environment compatibility', () => { + describe('Object.setPrototypeOf', () => { + it('does not throw if unavailable', () => { + // arrange + Environment.getSetPrototypeOf = () => undefined; + + // act + const act = () => new CustomErrorConcrete(); + + // assert + expect(act).to.not.throw(); + }); + it('calls if available', () => { + // arrange + let wasCalled = false; + const setPrototypeOf = () => { wasCalled = true; }; + Environment.getSetPrototypeOf = () => setPrototypeOf; + + // act + // eslint-disable-next-line no-new + new CustomErrorConcrete(); + + // assert + expect(wasCalled).to.equal(true); + }); + }); + describe('Error.captureStackTrace', () => { + it('does not throw if unavailable', () => { + // arrange + Environment.getCaptureStackTrace = () => undefined; + + // act + const act = () => new CustomErrorConcrete(); + + // assert + expect(act).to.not.throw(); + }); + it('calls if available', () => { + // arrange + let wasCalled = false; + const captureStackTrace = () => { wasCalled = true; }; + Environment.getCaptureStackTrace = () => captureStackTrace; + + // act + // eslint-disable-next-line no-new + new CustomErrorConcrete(); + + // assert + expect(wasCalled).to.equal(true); + }); + }); + }); + describe('runtime behavior sanity checks', () => { + /* + * These tests are intended to verify the behavior of the JavaScript runtime or environment, + * rather than specific application logic. Typically, we avoid such tests because we + * trust the behavior of the underlying platform. However, they've been included here + * due to previous unexpected issues, specifically failures when trying to log + * `new Error().stack`. These issues arose because of factors like transpilation, + * source-mapping, and variances in JavaScript engine behaviors. + */ + it('`Error.stack` is defined', () => { + const error = new Error(); + + // act + const { stack } = error; + + // assert + expect(stack).to.not.equal(undefined); + }); + }); +}); + +class CustomErrorConcrete extends CustomError { } diff --git a/tests/unit/application/Parser/NodeValidation/NodeDataError.spec.ts b/tests/unit/application/Parser/NodeValidation/NodeDataError.spec.ts index 1b2af9b..18f9873 100644 --- a/tests/unit/application/Parser/NodeValidation/NodeDataError.spec.ts +++ b/tests/unit/application/Parser/NodeValidation/NodeDataError.spec.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { INodeDataErrorContext, NodeDataError } from '@/application/Parser/NodeValidation/NodeDataError'; import { NodeDataErrorContextStub } from '@tests/unit/shared/Stubs/NodeDataErrorContextStub'; import { NodeType } from '@/application/Parser/NodeValidation/NodeType'; +import { CustomError } from '@/application/Common/CustomError'; describe('NodeDataError', () => { it('sets message as expected', () => { @@ -28,20 +29,13 @@ describe('NodeDataError', () => { // assert expect(sut.context).to.equal(expected); }); - it('sets stack as expected', () => { + it('extends CustomError', () => { // arrange + const expected = CustomError; // act const sut = new NodeDataErrorBuilder() .build(); // assert - expect(sut.stack !== undefined); - }); - it('extends Error', () => { - // arrange - const expected = Error; - // act - const sut = new NodeDataErrorBuilder().build(); - // assert expect(sut).to.be.an.instanceof(expected); }); }); diff --git a/vue.config.js b/vue.config.js index 12b7544..d3caecd 100644 --- a/vue.config.js +++ b/vue.config.js @@ -24,6 +24,9 @@ module.exports = defineConfig({ }, // Fix compilation failing on macOS when running unit/integration tests externals: ['fsevents'], + // Use something other than default source mapper or babel cannot + // log stacks like `console.log(new Error().stack)` + devtool: 'eval-source-map', }, pluginOptions: { // https://nklayman.github.io/vue-cli-plugin-electron-builder/guide/guide.html#native-modules