Skip to content

Commit

Permalink
feat: let cause be unknown for constructor, create (like wrap)
Browse files Browse the repository at this point in the history
  • Loading branch information
mshanemc committed Apr 5, 2024
1 parent 39bd401 commit aed3f3c
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 33 deletions.
6 changes: 3 additions & 3 deletions src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,14 @@ export class Messages<T extends string> {
}

if (!packageName) {
const errMessage = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
const message = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
try {
packageName = asString(ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name);
if (!packageName) {
throw new NamedError('MissingPackageName', errMessage);
throw SfError.create({ message, name: 'MissingPackageName' });
}
} catch (err) {
throw new NamedError('MissingPackageName', errMessage, err as Error);
throw SfError.create({ message, name: 'MissingPackageName', cause: err });
}
}

Expand Down
25 changes: 11 additions & 14 deletions src/sfError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export type SfErrorOptions<T extends ErrorDataProperties = ErrorDataProperties>
exitCode?: number;
name?: string;
data?: T;
cause?: Error;
/** pass an Error. For convenience in catch blocks, code will check that it is, in fact, an Error */
cause?: unknown;
context?: string;
actions?: string[];
};
Expand Down Expand Up @@ -74,12 +75,14 @@ export class SfError<T extends ErrorDataProperties = ErrorDataProperties> extend
name = 'SfError',
actions?: string[],
exitCodeOrCause?: number | Error,
cause?: Error
cause?: unknown
) {
const derivedCause = exitCodeOrCause instanceof Error ? exitCodeOrCause : cause;
if (typeof cause !== 'undefined' && !(cause instanceof Error)) {
throw new TypeError(`The cause, if provided, must be an instance of Error. Received: ${typeof cause}`);
}
super(message);
this.name = name;
this.cause = derivedCause;
this.cause = exitCodeOrCause instanceof Error ? exitCodeOrCause : cause;
this.actions = actions;
if (typeof exitCodeOrCause === 'number') {
this.exitCode = exitCodeOrCause;
Expand All @@ -88,12 +91,7 @@ export class SfError<T extends ErrorDataProperties = ErrorDataProperties> extend
}
}

public get fullStack(): string | undefined {
return recursiveStack(this).join('\nCaused by: ');
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
public get code(): any {
public get code(): string {
return this.#code ?? this.name;
}

Expand Down Expand Up @@ -127,7 +125,9 @@ export class SfError<T extends ErrorDataProperties = ErrorDataProperties> extend
? // a basic error with message and name. We make it the cause to preserve any other properties
new SfError<T>(err.message, err.name, undefined, err)
: // ok, something was throws that wasn't error or string. Convert it to an Error that preserves the information as the cause and wrap that.
SfError.wrap<T>(new TypeError('An unexpected error occurred', { cause: err }));
SfError.wrap<T>(
new TypeError(`SfError.wrap received type ${typeof err} but expects type Error or string`, { cause: err })
);

// If the original error has a code, use that instead of name.
if (hasString(err, 'code')) {
Expand Down Expand Up @@ -171,6 +171,3 @@ export class SfError<T extends ErrorDataProperties = ErrorDataProperties> extend
};
}
}

const recursiveStack = (err: Error): string[] =>
(err.cause && err.cause instanceof Error ? [err.stack, ...recursiveStack(err.cause)] : [err.stack]).filter(isString);
4 changes: 2 additions & 2 deletions test/unit/org/scratchOrgInfoApiTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ describe('requestScratchOrgCreation', () => {
expect.fail('should have thrown SfError');
}
expect(error).to.exist;
expect(error).to.have.keys(['cause', 'name', 'actions', 'exitCode']);
expect(error).to.include.keys(['cause', 'name', 'actions', 'exitCode']);
expect((error as Error).toString()).to.include('SignupDuplicateSettingsSpecifiedError');
}
});
Expand All @@ -216,7 +216,7 @@ describe('requestScratchOrgCreation', () => {
expect.fail('should have thrown SfError');
}
expect(error).to.exist;
expect(error).to.have.keys(['cause', 'name', 'actions', 'exitCode']);
expect(error).to.include.keys(['cause', 'name', 'actions', 'exitCode']);
expect((error as Error).toString()).to.include(messages.getMessage('DeprecatedPrefFormat'));
}
});
Expand Down
38 changes: 24 additions & 14 deletions test/unit/sfErrorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { inspect } from 'node:util';
import { expect, assert } from 'chai';
import { Messages } from '../../src/messages';
import { SfError } from '../../src/sfError';

Messages.importMessageFile('pname', 'testMessages.json');

const causeDelimiter = 'cause:';

describe('SfError', () => {
describe('constructor', () => {
it('should return a mutable SfError', () => {
Expand Down Expand Up @@ -70,32 +73,38 @@ describe('SfError', () => {
});
});

describe('fullStack', () => {
describe('nested errors', () => {
const causeRegex = new RegExp(causeDelimiter, 'g');
const nestedCauseRegex = new RegExp(/\[cause:\]/, 'g');
it('returned `name:message` when no cause', () => {
const err = new SfError('test');
expect(err.fullStack).to.include('SfError: test');
expect(err.fullStack).to.include('sfErrorTest.ts');
expect(err.fullStack).to.not.include('Caused by:');
expect(inspect(err)).to.include('SfError: test');
expect(inspect(err)).to.include('sfErrorTest.ts');
// there's always 1 cause from the `cause:` property, even if undefined
expect(inspect(err)?.match(causeRegex)).to.have.lengthOf(1);
});
it('1 cause', () => {
const nestedError = new Error('nested');
const err = new SfError('test', undefined, undefined, nestedError);
expect(err.fullStack).to.include('SfError: test');
expect(err.fullStack).to.include('sfErrorTest.ts');
expect(err.fullStack).to.include('nested');
expect(err.fullStack?.match(/Caused by:/g)).to.have.lengthOf(1);
expect(inspect(err)).to.include('SfError: test');
expect(inspect(err)).to.include('sfErrorTest.ts');
expect(inspect(err)).to.include('nested');
expect(inspect(err)?.match(causeRegex)).to.have.lengthOf(1);
expect(inspect(err)?.match(nestedCauseRegex)).to.be.null;
});
it('recurse through stacked causes', () => {
const nestedError = new Error('nested');
const nestedError2 = new Error('nested2', { cause: nestedError });
const err = new SfError('test', undefined, undefined, nestedError2);
expect(err.fullStack).to.include('SfError: test');
expect(err.fullStack).to.include('sfErrorTest.ts');
expect(err.fullStack).to.include('nested');
expect(err.fullStack).to.include('nested2');
expect(err.fullStack?.match(/Caused by:/g)).to.have.lengthOf(2);
expect(inspect(err)).to.include('SfError: test');
expect(inspect(err)).to.include('sfErrorTest.ts');
expect(inspect(err)).to.include('nested');
expect(inspect(err)).to.include('nested2');
expect(inspect(err)?.match(causeRegex)).to.have.lengthOf(1);
expect(inspect(err)?.match(causeRegex)).to.have.lengthOf(1);
});
});

describe('wrap', () => {
it('should return a wrapped error', () => {
const myErrorMsg = 'yikes! What did you do?';
Expand All @@ -106,7 +115,8 @@ describe('SfError', () => {
expect(mySfError).to.be.an.instanceOf(SfError);
expect(mySfError.message).to.equal(myErrorMsg);
expect(mySfError.name).to.equal(myErrorName);
expect(mySfError.fullStack).to.contain('Caused by:').and.contain(myError.stack);
expect(mySfError.cause).to.equal(myError);
expect(inspect(mySfError)).to.contain(causeDelimiter).and.contain(myErrorMsg);
});

it('should return a wrapped error with a code', () => {
Expand Down

3 comments on commit aed3f3c

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: aed3f3c Previous: 092af1b Ratio
Child logger creation 475152 ops/sec (±1.29%) 463768 ops/sec (±2.81%) 0.98
Logging a string on root logger 738890 ops/sec (±5.89%) 844629 ops/sec (±6.74%) 1.14
Logging an object on root logger 540081 ops/sec (±8.93%) 638628 ops/sec (±5.42%) 1.18
Logging an object with a message on root logger 16046 ops/sec (±188.10%) 5889 ops/sec (±215.33%) 0.37
Logging an object with a redacted prop on root logger 393676 ops/sec (±12.13%) 411224 ops/sec (±12.94%) 1.04
Logging a nested 3-level object on root logger 342867 ops/sec (±9.78%) 420709 ops/sec (±6.36%) 1.23

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - windows-latest

Benchmark suite Current: aed3f3c Previous: 092af1b Ratio
Child logger creation 329489 ops/sec (±0.57%) 341561 ops/sec (±0.49%) 1.04
Logging a string on root logger 776306 ops/sec (±6.24%) 832859 ops/sec (±11.31%) 1.07
Logging an object on root logger 618335 ops/sec (±6.61%) 673870 ops/sec (±5.28%) 1.09
Logging an object with a message on root logger 7206 ops/sec (±203.12%) 21982 ops/sec (±183.16%) 3.05
Logging an object with a redacted prop on root logger 418398 ops/sec (±14.11%) 489716 ops/sec (±6.91%) 1.17
Logging a nested 3-level object on root logger 336244 ops/sec (±5.29%) 326489 ops/sec (±5.51%) 0.97

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - windows-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: aed3f3c Previous: 092af1b Ratio
Logging an object with a message on root logger 7206 ops/sec (±203.12%) 21982 ops/sec (±183.16%) 3.05

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.