44r0n7/privacy.sexy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve compiler error display for latest Chromium

Browse Source 
This commit addresses the issue of Chromium v126 and later not displaying
error messages correctly when the error object's `message` property uses
a getter. It refactors the code to utilize an immutable Error object with
recursive context, improves error message formatting and leverages the
`cause` property.

Changes:

- Refactor error wrapping internals to use an immutable error object,
  eliminating `message` getters.
- Utilize the `cause` property in contextual errors for enhanced error
  display in the console.
- Enhance message formatting with better indentation and listing.
- Improve clarity by renaming values thrown during validations.
This commit is contained in:
undergroundwires
2024-07-21 10:18:27 +02:00
parent abe03cef3f
commit b16e13678c
15 changed files with 253 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/application/Parser/ApplicationParser.ts
View File

@@ -31,7 +31,7 @@ function validateCollectionsData(
) {
validator.assertNonEmptyCollection({
value: collections,
valueName: 'collections',
valueName: 'Collections',
});
}

4
src/application/Parser/CategoryCollectionParser.ts
View File

@@ -45,14 +45,14 @@ function validateCollection(
): void {
validator.assertObject({
value: content,
valueName: 'collection',
valueName: 'Collection',
allowedProperties: [
'os', 'scripting', 'actions', 'functions',
],
});
validator.assertNonEmptyCollection({
value: content.actions,
valueName: '"actions" in collection',
valueName: '\'actions\' in collection',
});
}

112
src/application/Parser/Common/ContextualError.ts
View File

@@ -1,42 +1,116 @@
import { CustomError } from '@/application/Common/CustomError';
import { indentText } from '@/application/Common/Text/IndentText';
export interface ErrorWithContextWrapper {
(
error: Error,
innerError: Error,
additionalContext: string,
): Error;
}
export const wrapErrorWithAdditionalContext: ErrorWithContextWrapper = (
error: Error,
additionalContext: string,
innerError,
additionalContext,
) => {
return (error instanceof ContextualError ? error : new ContextualError(error))
.withAdditionalContext(additionalContext);
if (!additionalContext) {
throw new Error('Missing additional context');
}
return new ContextualError({
innerError,
additionalContext,
});
};
/* AggregateError is similar but isn't well-serialized or displayed by browsers */
/**
* Class for building a detailed error trace.
*
* Alternatives considered:
* - `AggregateError`:
* Similar but not well-serialized or displayed by browsers such as Chromium (last tested v126).
* - `cause` property:
* Not displayed by all browsers (last tested v126).
* Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
*
* This is immutable where the constructor sets the values because using getter functions such as
* `get cause()`, `get message()` does not work on Chromium (last tested v126), but works fine on
* Firefox (last tested v127).
*/
class ContextualError extends CustomError {
private readonly additionalContext = new Array<string>();
constructor(
public readonly innerError: Error,
) {
super();
constructor(public readonly context: ErrorContext) {
super(
generateDetailedErrorMessageWithContext(context),
{
cause: context.innerError,
},
);
}
}
public withAdditionalContext(additionalContext: string): this {
this.additionalContext.push(additionalContext);
return this;
interface ErrorContext {
readonly innerError: Error;
readonly additionalContext: string;
}
public get message(): string { // toString() is not used when Chromium logs it on console
function generateDetailedErrorMessageWithContext(
context: ErrorContext,
): string {
return [
'\n',
this.innerError.message,
// Display the current error message first, then the root cause.
// This prevents repetitive main messages for errors with a `cause:` chain,
// aligning with browser error display conventions.
context.additionalContext,
'\n',
'Error Trace (starting from root cause):',
indentText(
formatErrorTrace(
// Displaying contexts from the top frame (deepest, most recent) aligns with
// common debugger/compiler standard.
extractErrorTraceAscendingFromDeepest(context),
),
),
'\n',
'Additional context:',
...this.additionalContext.map((context, index) => `${index + 1}: ${context}`),
].join('\n');
}
function extractErrorTraceAscendingFromDeepest(
context: ErrorContext,
): string[] {
const originalError = findRootError(context.innerError);
const contextsDescendingFromMostRecent: string[] = [
context.additionalContext,
...gatherContextsFromErrorChain(context.innerError),
originalError.toString(),
];
const contextsAscendingFromDeepest = contextsDescendingFromMostRecent.reverse();
return contextsAscendingFromDeepest;
}
function findRootError(error: Error): Error {
if (error instanceof ContextualError) {
return findRootError(error.context.innerError);
}
return error;
}
function gatherContextsFromErrorChain(
error: Error,
accumulatedContexts: string[] = [],
): string[] {
if (error instanceof ContextualError) {
accumulatedContexts.push(error.context.additionalContext);
return gatherContextsFromErrorChain(error.context.innerError, accumulatedContexts);
}
return accumulatedContexts;
}
function formatErrorTrace(
errorMessages: readonly string[],
): string {
if (errorMessages.length === 1) {
return errorMessages[0];
}
return errorMessages
.map((context, index) => `${index + 1}.${indentText(context)}`)
.join('\n');
}

4
src/application/Parser/Common/TypeValidator.ts
View File

@@ -108,7 +108,7 @@ function assertArray(
valueName: string,
): asserts value is Array<unknown> {
if (!isArray(value)) {
throw new Error(`'${valueName}' should be of type 'array', but is of type '${typeof value}'.`);
throw new Error(`${valueName} should be of type 'array', but is of type '${typeof value}'.`);
}
}
@@ -117,7 +117,7 @@ function assertString(
valueName: string,
): asserts value is string {
if (!isString(value)) {
throw new Error(`'${valueName}' should be of type 'string', but is of type '${typeof value}'.`);
throw new Error(`${valueName} should be of type 'string', but is of type '${typeof value}'.`);
}
}

2
src/application/Parser/Executable/CategoryParser.ts
View File

@@ -84,7 +84,7 @@ function ensureValidCategory(
});
validator.assertType((v) => v.assertObject({
value: category,
valueName: category.category ?? 'category',
valueName: `Category '${category.category}'` ?? 'Category',
allowedProperties: [
'docs', 'children', 'category',
],

2
src/application/Parser/Executable/Script/Compiler/Function/Call/Argument/FunctionCallArgument.ts
View File

@@ -22,7 +22,7 @@ export const createFunctionCallArgument: FunctionCallArgumentFactory = (
utilities.validateParameterName(parameterName);
utilities.typeValidator.assertNonEmptyString({
value: argumentValue,
valueName: `Missing argument value for the parameter "${parameterName}".`,
valueName: `Function parameter '${parameterName}'`,
});
return {
parameterName,

4
src/application/Parser/Executable/Script/Compiler/Function/Call/FunctionCallsParser.ts
View File

@@ -42,7 +42,7 @@ function getCallSequence(calls: FunctionCallsData, validator: TypeValidator): Fu
if (isArray(calls)) {
validator.assertNonEmptyCollection({
value: calls,
valueName: 'function call sequence',
valueName: 'Function call sequence',
});
return calls as FunctionCallData[];
}
@@ -56,7 +56,7 @@ function parseFunctionCall(
): FunctionCall {
utilities.typeValidator.assertObject({
value: call,
valueName: 'function call',
valueName: 'Function call',
allowedProperties: ['function', 'parameters'],
});
const callArgs = parseArgs(call.parameters, utilities.createCallArgument);

2
src/application/Parser/Executable/Script/Compiler/Function/Shared/ParameterNameValidator.ts
View File

@@ -13,7 +13,7 @@ export const validateParameterName = (
) => {
typeValidator.assertNonEmptyString({
value: parameterName,
valueName: 'parameter name',
valueName: 'Parameter name',
rule: {
expectedMatch: /^[0-9a-zA-Z]+$/,
errorMessage: `parameter name must be alphanumeric but it was "${parameterName}".`,

2
src/application/Parser/Executable/Script/ScriptParser.ts
View File

@@ -102,7 +102,7 @@ function validateScript(
): asserts script is NonNullable<ScriptData> {
validator.assertType((v) => v.assertObject<CallScriptData & CodeScriptData>({
value: script,
valueName: script.name ?? 'script',
valueName: `Script '${script.name}'` ?? 'Script',
allowedProperties: [
'name', 'recommend', 'code', 'revertCode', 'call', 'docs',
],

2
src/application/Parser/ScriptingDefinition/ScriptingDefinitionParser.ts
View File

@@ -37,7 +37,7 @@ function validateData(
): void {
validator.assertObject({
value: data,
valueName: 'scripting definition',
valueName: 'Scripting definition',
allowedProperties: ['language', 'fileExtension', 'startCode', 'endCode'],
});
}

2
tests/unit/application/Parser/ApplicationParser.spec.ts
View File

@@ -134,7 +134,7 @@ describe('ApplicationParser', () => {
const data = [new CollectionDataStub()];
const expectedAssertion: NonEmptyCollectionAssertion = {
value: data,
valueName: 'collections',
valueName: 'Collections',
};
const validator = new TypeValidatorStub();
const sut = new ApplicationParserBuilder()

4
tests/unit/application/Parser/CategoryCollectionParser.spec.ts
View File

@@ -28,7 +28,7 @@ describe('CategoryCollectionParser', () => {
const data = new CollectionDataStub();
const expectedAssertion: ObjectAssertion<CollectionData> = {
value: data,
valueName: 'collection',
valueName: 'Collection',
allowedProperties: [
'os', 'scripting', 'actions', 'functions',
],
@@ -48,7 +48,7 @@ describe('CategoryCollectionParser', () => {
const actions = [getCategoryStub('test1'), getCategoryStub('test2')];
const expectedAssertion: NonEmptyCollectionAssertion = {
value: actions,
valueName: '"actions" in collection',
valueName: '\'actions\' in collection',
};
const validator = new TypeValidatorStub();
const context = new TestContext()

191
tests/unit/application/Parser/Common/ContextualError.spec.ts
View File

@@ -1,101 +1,168 @@
import { describe, it, expect } from 'vitest';
import { CustomError } from '@/application/Common/CustomError';
import { wrapErrorWithAdditionalContext } from '@/application/Parser/Common/ContextualError';
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage';
import { indentText } from '@/application/Common/Text/IndentText';
import { itEachAbsentStringValue } from '@tests/unit/shared/TestCases/AbsentTests';
describe('wrapErrorWithAdditionalContext', () => {
it('preserves the original error when wrapped', () => {
// arrange
const expectedError = new Error();
const context = new TestContext()
.withError(expectedError);
// act
const error = context.wrap();
// assert
const actualError = extractInnerErrorFromContextualError(error);
expect(actualError).to.equal(expectedError);
});
it('maintains the original error when re-wrapped', () => {
// arrange
const expectedError = new Error();
// act
const firstError = new TestContext()
.withError(expectedError)
.withAdditionalContext('first error')
.wrap();
const secondError = new TestContext()
.withError(firstError)
.withAdditionalContext('second error')
.wrap();
// assert
const actualError = extractInnerErrorFromContextualError(secondError);
expect(actualError).to.equal(expectedError);
});
it(`the object extends ${CustomError.name}`, () => {
it(`extend ${CustomError.name}`, () => {
// arrange
const expected = CustomError;
// act
const error = new TestContext()
.wrap();
.build();
// assert
expect(error).to.be.an.instanceof(expected);
});
describe('inner error preservation', () => {
it('preserves the original error', () => {
// arrange
const expectedError = new Error();
const context = new TestContext()
.withInnerError(expectedError);
// act
const error = context.build();
// assert
const actualError = getInnerErrorFromContextualError(error);
expect(actualError).to.equal(expectedError);
});
it('sets the original error as the cause', () => {
// arrange
const expectedError = new Error('error causing the issue');
const context = new TestContext()
.withInnerError(expectedError);
// act
const error = context.build();
// assert
const actualError = error.cause;
expect(actualError).to.equal(expectedError);
});
});
describe('error message construction', () => {
it('includes the message from the original error', () => {
it('includes the original error message', () => {
// arrange
const expectedOriginalErrorMessage = 'Message from the inner error';
// act
const error = new TestContext()
.withError(new Error(expectedOriginalErrorMessage))
.wrap();
.withInnerError(new Error(expectedOriginalErrorMessage))
.build();
// assert
expect(error.message).contains(expectedOriginalErrorMessage);
});
it('appends provided additional context to the error message', () => {
it('includes original error toString() if message is absent', () => {
// arrange
const originalError = new Error();
const expectedPartInMessage = originalError.toString();
// act
const error = new TestContext()
.withInnerError(originalError)
.build();
// assert
expect(error.message).contains(expectedPartInMessage);
});
it('appends additional context to the error message', () => {
// arrange
const expectedAdditionalContext = 'Expected additional context message';
// act
const error = new TestContext()
.withAdditionalContext(expectedAdditionalContext)
.wrap();
.build();
// assert
expect(error.message).contains(expectedAdditionalContext);
});
it('appends multiple contexts to the error message in sequential order', () => {
describe('message order', () => {
it('displays the latest context before the original error message', () => {
// arrange
const expectedFirstContext = 'First context';
const expectedSecondContext = 'Second context';
const originalErrorMessage = 'Original message from the inner error to be shown first';
const additionalContext = 'Context to be displayed after';
// act
const firstError = new TestContext()
.withAdditionalContext(expectedFirstContext)
.wrap();
const secondError = new TestContext()
.withError(firstError)
.withAdditionalContext(expectedSecondContext)
.wrap();
const error = new TestContext()
.withInnerError(new Error(originalErrorMessage))
.withAdditionalContext(additionalContext)
.build();
// assert
const messageLines = splitTextIntoLines(secondError.message);
expect(messageLines).to.contain(`1: ${expectedFirstContext}`);
expect(messageLines).to.contain(`2: ${expectedSecondContext}`);
expectMessageDisplayOrder(error.message, {
firstMessage: additionalContext,
secondMessage: originalErrorMessage,
});
});
it('appends multiple contexts from most specific to most general', () => {
// arrange
const deepErrorContext = 'first-context';
const parentErrorContext = 'second-context';
// act
const deepError = new TestContext()
.withAdditionalContext(deepErrorContext)
.build();
const parentError = new TestContext()
.withInnerError(deepError)
.withAdditionalContext(parentErrorContext)
.build();
const grandParentError = new TestContext()
.withInnerError(parentError)
.withAdditionalContext('latest-error')
.build();
// assert
expectMessageDisplayOrder(grandParentError.message, {
firstMessage: deepErrorContext,
secondMessage: parentErrorContext,
});
});
});
});
describe('throws error when context is missing', () => {
itEachAbsentStringValue((absentValue) => {
// arrange
const expectedError = 'Missing additional context';
const context = new TestContext()
.withAdditionalContext(absentValue);
// act
const act = () => context.build();
// assert
expect(act).to.throw(expectedError);
}, { excludeNull: true, excludeUndefined: true });
});
});
function expectMessageDisplayOrder(
actualMessage: string,
expectation: {
readonly firstMessage: string;
readonly secondMessage: string;
},
): void {
const firstMessageIndex = actualMessage.indexOf(expectation.firstMessage);
const secondMessageIndex = actualMessage.indexOf(expectation.secondMessage);
expect(firstMessageIndex).to.be.lessThan(secondMessageIndex, formatAssertionMessage([
'Error output order does not match the expected order.',
'Expected the first message to be displayed before the second message.',
'Expected first message:',
indentText(expectation.firstMessage),
'Expected second message:',
indentText(expectation.secondMessage),
'Received message:',
indentText(actualMessage),
]));
}
class TestContext {
private error: Error = new Error();
private innerError: Error = new Error(`[${TestContext.name}] original error`);
private additionalContext = `[${TestContext.name}] additional context`;
public withError(error: Error) {
this.error = error;
public withInnerError(innerError: Error) {
this.innerError = innerError;
return this;
}
@@ -104,19 +171,21 @@ class TestContext {
return this;
}
public wrap(): ReturnType<typeof wrapErrorWithAdditionalContext> {
public build(): ReturnType<typeof wrapErrorWithAdditionalContext> {
return wrapErrorWithAdditionalContext(
this.error,
this.innerError,
this.additionalContext,
);
}
}
function extractInnerErrorFromContextualError(error: Error): Error {
const innerErrorProperty = 'innerError';
if (!(innerErrorProperty in error)) {
throw new Error(`${innerErrorProperty} property is missing`);
function getInnerErrorFromContextualError(error: Error & {
readonly context?: {
readonly innerError?: Error;
},
}): Error {
if (error?.context?.innerError instanceof Error) {
return error.context.innerError;
}
const actualError = error[innerErrorProperty];
return actualError as Error;
throw new Error('Error must have a context with a valid innerError property.');
}

2
tests/unit/application/Parser/Common/TypeValidator.spec.ts
View File

@@ -217,7 +217,7 @@ describe('createTypeValidator', () => {
}) => {
it(description, () => {
const valueName = 'invalidValue';
const expectedMessage = `'${valueName}' should be of type 'string', but is of type '${typeof invalidValue}'.`;
const expectedMessage = `${valueName} should be of type 'string', but is of type '${typeof invalidValue}'.`;
const { assertNonEmptyString } = createTypeValidator();
// act
const act = () => assertNonEmptyString({ value: invalidValue, valueName });

6
tests/unit/application/Parser/Executable/Script/Compiler/Function/Call/FunctionCallsParser.spec.ts
View File

@@ -57,7 +57,7 @@ describe('FunctionCallsParser', () => {
const data = new FunctionCallDataStub();
const expectedAssertion: ObjectAssertion<FunctionCallData> = {
value: data,
valueName: 'function call',
valueName: 'Function call',
allowedProperties: [
'function', 'parameters',
],
@@ -117,7 +117,7 @@ describe('FunctionCallsParser', () => {
const data: FunctionCallsData = [new FunctionCallDataStub()];
const expectedAssertion: NonEmptyCollectionAssertion = {
value: data,
valueName: 'function call sequence',
valueName: 'Function call sequence',
};
const validator = new TypeValidatorStub();
const context = new TestContext()
@@ -134,7 +134,7 @@ describe('FunctionCallsParser', () => {
const data: FunctionCallsData = [expectedValidatedCallData];
const expectedAssertion: ObjectAssertion<FunctionCallData> = {
value: expectedValidatedCallData,
valueName: 'function call',
valueName: 'Function call',
allowedProperties: [
'function', 'parameters',
],