Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid error class name mangling in webpack #4768

Merged
merged 4 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions src/errors/errorHelpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import {
} from "@/errors/businessErrors";
import { ContextError } from "@/errors/genericErrors";
import {
ClientNetworkPermissionError,
ClientRequestError,
RemoteServiceError,
} from "@/errors/clientRequestErrors";
Expand Down Expand Up @@ -500,49 +499,3 @@ describe("serialization", () => {
expect(hasSpecificErrorCause(error, CancelError)).toBeTrue();
});
});

describe("robust to name mangling", () => {
it("handles namespaced business errors", () => {
class businessErrors_BusinessError extends Error {
// The name that webpack will produce during optimize.concatenateModules
override name = "businessErrors_BusinessError";
}

expect(
hasSpecificErrorCause(
new CancelError("test"),
businessErrors_BusinessError
)
).toBeTrue();
});

it("handles namespaced business errors in a context error", () => {
class businessErrors_BusinessError extends Error {
// The name that webpack will produce during optimize.concatenateModules
override name = "businessErrors_BusinessError";
}

const error = new ContextError("foo", {
cause: new CancelError("test"),
context: {},
});

expect(
hasSpecificErrorCause(error, businessErrors_BusinessError)
).toBeTrue();
});

it("handles namespaced client request errors", () => {
class clientRequestErrors_ClientRequestError extends Error {
// The name that webpack will produce during optimize.concatenateModules
override name = "clientRequestErrors_ClientRequestError";
}

expect(
hasSpecificErrorCause(
new ClientNetworkPermissionError("test", { cause: null }),
clientRequestErrors_ClientRequestError
)
).toBeTrue();
});
});
49 changes: 5 additions & 44 deletions src/errors/errorHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,15 @@ export function isSpecificError<
>(error: unknown, errorType: ErrorType): error is InstanceType<ErrorType> {
// Catch 2 common error subclass groups. Necessary until we drop support for serialized errors:
// https://github.com/sindresorhus/serialize-error/issues/72
if (isErrorTypeNameMatch(errorType.name, ["ClientRequestError"])) {
if (errorType.name === "ClientRequestError") {
return isClientRequestError(error);
}

if (isErrorTypeNameMatch(errorType.name, ["BusinessError"])) {
if (errorType.name === "BusinessError") {
return isBusinessError(error);
}

return (
isErrorObject(error) && isErrorTypeNameMatch(error.name, [errorType.name])
);
return isErrorObject(error) && error.name === errorType.name;
}

export function selectSpecificError<
Expand Down Expand Up @@ -174,42 +172,8 @@ const BUSINESS_ERROR_NAMES = new Set([
"InvalidSelectorError",
]);

/**
* Returns true if errorName matches at least one of classNames.
*
* Accounts for name-mangling by webpack in optimized code for the class names in classNames.
*
* @param errorName the query error name
* @param classNames the class names to match against
*/
export function isErrorTypeNameMatch(
errorName: string,
classNames: Iterable<string>
): boolean {
// https://github.com/pixiebrix/pixiebrix-extension/issues/4763

// In production builds, webpack tries to hoist classes to the global scope. To do so, it namespaces the class name
// with the module name: https://webpack.js.org/configuration/optimization/#optimizationconcatenatemodules

// Also note that keep_classnames must be set in webpack's TerserPlugin configuration to preserve the error
// class names for the name check to work

return (
errorName &&
// Defensive check because some call sites cast from unknown
typeof errorName === "string" &&
[...classNames].some(
(className) =>
errorName === className || errorName.endsWith(`_${className}`)
)
);
}

export function isBusinessError(error: unknown): boolean {
return (
isErrorObject(error) &&
isErrorTypeNameMatch(error.name, BUSINESS_ERROR_NAMES)
);
return isErrorObject(error) && BUSINESS_ERROR_NAMES.has(error.name);
}

// List all ClientRequestError subclasses as text:
Expand All @@ -227,10 +191,7 @@ const CLIENT_REQUEST_ERROR_NAMES = new Set([
* @see CLIENT_REQUEST_ERROR_NAMES
*/
export function isClientRequestError(error: unknown): boolean {
return (
isErrorObject(error) &&
isErrorTypeNameMatch(error.name, CLIENT_REQUEST_ERROR_NAMES)
);
return isErrorObject(error) && CLIENT_REQUEST_ERROR_NAMES.has(error.name);
}

/**
Expand Down
4 changes: 1 addition & 3 deletions src/errors/networkErrorHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { type Except } from "type-fest";
import { type AxiosError, type AxiosResponse } from "axios";
import { isEmpty, isPlainObject } from "lodash";
import { getReasonPhrase } from "http-status-codes";
import { isErrorTypeNameMatch } from "@/errors/errorHelpers";

/**
* Axios offers its own serialization method, but it doesn't include the response.
Expand All @@ -35,8 +34,7 @@ export function isAxiosError(error: unknown): error is SerializableAxiosError {
// To deal with original AxiosError as well as a serialized error
// we check 'isAxiosError' property for a non-serialized Error and 'name' for serialized object
// Related issue to revisit RTKQ error handling: https://github.com/pixiebrix/pixiebrix-extension/issues/4032
(Boolean(error.isAxiosError) ||
isErrorTypeNameMatch(error.name as string, ["AxiosError"]))
(Boolean(error.isAxiosError) || error.name === "AxiosError")
);
}

Expand Down
3 changes: 3 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ module.exports = (env, options) =>
},

optimization: {
// Module concatenation mangles class names https://github.com/pixiebrix/pixiebrix-extension/issues/4763
concatenateModules: false,

// Chrome bug https://bugs.chromium.org/p/chromium/issues/detail?id=1108199
splitChunks: {
automaticNameDelimiter: "-",
Expand Down