Skip to content

Commit

Permalink
⚡️ Delay computation of Error stack when no cause (#4472)
Browse files Browse the repository at this point in the history
**💥 Breaking change**

Up-to-now fast-check was always sealing the stack attached to errors as they popped. With error with cause capabilities we started to draft the required links to pass the instance of Error up to the end. We can thus safely delay the computation of the stack.

As delaying might have unwanted side-effects (maybe our instance of Error would be touched), we prefer not to make this change as part of a minor release.

Over all the breaking is justified by the change we do on the API of the failures. We dropped the previously precomputed errors and only keep the raw instance of errors. Users now have to manipulate it to extract the relevant error if needed.

First piece for #4416
  • Loading branch information
dubzzz authored Aug 13, 2024
1 parent 5da13b0 commit 2684a00
Show file tree
Hide file tree
Showing 20 changed files with 737 additions and 88 deletions.
12 changes: 3 additions & 9 deletions packages/fast-check/src/check/property/AsyncProperty.generic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
noUndefinedAsContext,
UndefinedContextPlaceholder,
} from '../../arbitrary/_internals/helpers/NoUndefinedAsContext';
import { Error, String } from '../../utils/globals';
import { Error } from '../../utils/globals';

/**
* Type of legal hook function that can be used to call `beforeEach` or `afterEach`
Expand Down Expand Up @@ -119,18 +119,12 @@ export class AsyncProperty<Ts> implements IAsyncPropertyWithHooks<Ts> {
const output = await this.predicate(v);
return output === undefined || output === true
? null
: {
error: new Error('Property failed by returning false'),
errorMessage: 'Error: Property failed by returning false',
};
: { error: new Error('Property failed by returning false') };
} catch (err) {
// precondition failure considered as success for the first version
if (PreconditionFailure.isFailure(err)) return err;
// exception as PropertyFailure in case of real failure
if (err instanceof Error && err.stack) {
return { error: err, errorMessage: err.stack }; // stack includes the message
}
return { error: err, errorMessage: String(err) };
return { error: err };
}
}

Expand Down
5 changes: 0 additions & 5 deletions packages/fast-check/src/check/property/IRawProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ export type PropertyFailure = {
* @remarks Since 3.0.0
*/
error: unknown;
/**
* The error message extracted from the error
* @remarks Since 3.0.0
*/
errorMessage: string;
};

/**
Expand Down
12 changes: 3 additions & 9 deletions packages/fast-check/src/check/property/Property.generic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
noUndefinedAsContext,
UndefinedContextPlaceholder,
} from '../../arbitrary/_internals/helpers/NoUndefinedAsContext';
import { Error, String } from '../../utils/globals';
import { Error } from '../../utils/globals';

/**
* Type of legal hook function that can be used to call `beforeEach` or `afterEach`
Expand Down Expand Up @@ -135,18 +135,12 @@ export class Property<Ts> implements IProperty<Ts>, IPropertyWithHooks<Ts> {
const output = this.predicate(v);
return output === undefined || output === true
? null
: {
error: new Error('Property failed by returning false'),
errorMessage: 'Error: Property failed by returning false',
};
: { error: new Error('Property failed by returning false') };
} catch (err) {
// precondition failure considered as success for the first version
if (PreconditionFailure.isFailure(err)) return err;
// exception as PropertyFailure in case of real failure
if (err instanceof Error && err.stack) {
return { error: err, errorMessage: err.stack }; // stack includes the message
}
return { error: err, errorMessage: String(err) };
return { error: err };
}
}

Expand Down
5 changes: 1 addition & 4 deletions packages/fast-check/src/check/property/TimeoutProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ const timeoutAfter = (timeMs: number, setTimeoutSafe: typeof setTimeout, clearTi
let timeoutHandle: ReturnType<typeof setTimeout> | null = null;
const promise = new Promise<PropertyFailure>((resolve) => {
timeoutHandle = setTimeoutSafe(() => {
resolve({
error: new Error(`Property timeout: exceeded limit of ${timeMs} milliseconds`),
errorMessage: `Property timeout: exceeded limit of ${timeMs} milliseconds`,
});
resolve({ error: new Error(`Property timeout: exceeded limit of ${timeMs} milliseconds`) });
}, timeMs);
});
return {
Expand Down
9 changes: 0 additions & 9 deletions packages/fast-check/src/check/runner/reporter/RunDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export interface RunDetailsFailureProperty<Ts> extends RunDetailsCommon<Ts> {
interrupted: boolean;
counterexample: Ts;
counterexamplePath: string;
error: string;
errorInstance: unknown;
}

Expand All @@ -48,7 +47,6 @@ export interface RunDetailsFailureTooManySkips<Ts> extends RunDetailsCommon<Ts>
interrupted: false;
counterexample: null;
counterexamplePath: null;
error: null;
errorInstance: null;
}

Expand All @@ -66,7 +64,6 @@ export interface RunDetailsFailureInterrupted<Ts> extends RunDetailsCommon<Ts> {
interrupted: true;
counterexample: null;
counterexamplePath: null;
error: null;
errorInstance: null;
}

Expand All @@ -83,7 +80,6 @@ export interface RunDetailsSuccess<Ts> extends RunDetailsCommon<Ts> {
interrupted: boolean;
counterexample: null;
counterexamplePath: null;
error: null;
errorInstance: null;
}

Expand Down Expand Up @@ -138,11 +134,6 @@ export interface RunDetailsCommon<Ts> {
* @remarks Since 0.0.7
*/
counterexample: Ts | null;
/**
* In case of failure: it contains the reason of the failure
* @remarks Since 0.0.7
*/
error: string | null;
/**
* In case of failure: it contains the error that has been thrown if any
* @remarks Since 3.0.0
Expand Down
2 changes: 0 additions & 2 deletions packages/fast-check/src/check/runner/reporter/RunExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ export class RunExecution<Ts> {
// Rq: Same as this.value
// => this.failure !== undefined
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
error: this.failure!.errorMessage,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
errorInstance: this.failure!.error,
failures: this.extractFailures(),
executionSummary: this.rootExecutionTrees,
Expand Down
42 changes: 40 additions & 2 deletions packages/fast-check/src/check/runner/utils/RunDetailsFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Error, safePush, safeReplace } from '../../../utils/globals';
import { Error, safeErrorToString, safePush, safeReplace, safeToString, String } from '../../../utils/globals';
import { stringify, possiblyAsyncStringify } from '../../../utils/stringify';
import { VerbosityLevel } from '../configuration/VerbosityLevel';
import { ExecutionStatus } from '../reporter/ExecutionStatus';
Expand Down Expand Up @@ -79,10 +79,48 @@ function preFormatTooManySkipped<Ts>(out: RunDetailsFailureTooManySkips<Ts>, str
return { message, details, hints };
}

/** @internal */
function prettyError(errorInstance: unknown) {
// Print the Error message and its associated stacktrace
if (errorInstance instanceof Error && errorInstance.stack !== undefined) {
return errorInstance.stack; // stack includes the message
}

// First fallback: String(.)
try {
return String(errorInstance);
} catch (_err) {
// no-op
}

// Second fallback: Error::toString()
if (errorInstance instanceof Error) {
try {
return safeErrorToString(errorInstance);
} catch (_err) {
// no-op
}
}

// Third fallback: Object::toString()
if (errorInstance !== null && typeof errorInstance === 'object') {
try {
return safeToString(errorInstance);
} catch (_err) {
// no-op
}
}

// Final fallback: Hardcoded string
return 'Failed to serialize errorInstance';
}

/** @internal */
function preFormatFailure<Ts>(out: RunDetailsFailureProperty<Ts>, stringifyOne: (value: Ts) => string) {
const noErrorInMessage = out.runConfiguration.errorWithCause;
const messageErrorPart = noErrorInMessage ? '' : `\nGot ${safeReplace(out.error, /^Error: /, 'error: ')}`;
const messageErrorPart = noErrorInMessage
? ''
: `\nGot ${safeReplace(prettyError(out.errorInstance), /^Error: /, 'error: ')}`;
const message = `Property failed after ${out.numRuns} tests\n{ seed: ${out.seed}, path: "${
out.counterexamplePath
}", endOnFailure: true }\nCounterexample: ${stringifyOne(out.counterexample)}\nShrunk ${
Expand Down
7 changes: 7 additions & 0 deletions packages/fast-check/src/utils/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,3 +448,10 @@ export function safeHasOwnProperty(instance: unknown, v: PropertyKey): boolean {
export function safeToString(instance: unknown): string {
return safeApply(untouchedToString, instance, []);
}

// Error

const untouchedErrorToString = Error.prototype.toString;
export function safeErrorToString(instance: Error): string {
return safeApply(untouchedErrorToString, instance, []);
}
7 changes: 2 additions & 5 deletions packages/fast-check/test/e2e/AsyncScheduler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe(`AsyncScheduler (seed: ${seed})`, () => {
);
// Node <16: Cannot read property 'toLowerCase' of undefined
// Node >=16: TypeError: Cannot read properties of undefined (reading 'toLowerCase')
expect(out.error).toContain(`'toLowerCase'`);
expect((out.errorInstance as Error).message).toContain(`'toLowerCase'`);
});

it('should detect race conditions leading to infinite loops', async () => {
Expand Down Expand Up @@ -175,10 +175,7 @@ describe(`AsyncScheduler (seed: ${seed})`, () => {
expect(outRetry.failed).toBe(true);
expect(outRetry.numRuns).toBe(1);

const cleanError = (error: string) => {
return error.replace(/AsyncScheduler\.spec\.ts:\d+:\d+/g, 'AsyncScheduler.spec.ts:*:*');
};
expect(cleanError(outRetry.error!)).toBe(cleanError(out.error!));
expect(outRetry.errorInstance).toStrictEqual(out.errorInstance);
expect(String(outRetry.counterexample![0])).toBe(String(out.counterexample![0]));
});
});
Loading

0 comments on commit 2684a00

Please sign in to comment.