diff --git a/src/background/logging.ts b/src/background/logging.ts index 616c701adc..51f6436c8f 100644 --- a/src/background/logging.ts +++ b/src/background/logging.ts @@ -32,7 +32,7 @@ import { IGNORED_ERROR_PATTERNS, isAxiosError, isContextError, - serializePixiebrixError, + serializeErrorAndProperties, } from "@/errors"; import { expectContext, forbidContext } from "@/utils/expectContext"; import { isAppRequest, selectAbsoluteUrl } from "@/services/requestErrorUtils"; @@ -332,9 +332,9 @@ export async function recordError( context: flatContext, message, data, - - // Ensure it's serialized - error: serializePixiebrixError(maybeSerializedError as Error), + // Ensure the object is fully serialized. Required because it will be stored in IDB and flow through the Redux + // state. Can be converted to serializeError after https://github.com/sindresorhus/serialize-error/issues/74 + error: serializeErrorAndProperties(maybeSerializedError), }), ]); } catch (recordError) { diff --git a/src/blocks/transformers/jq.test.ts b/src/blocks/transformers/jq.test.ts new file mode 100644 index 0000000000..004e962226 --- /dev/null +++ b/src/blocks/transformers/jq.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2022 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { JQTransformer } from "@/blocks/transformers/jq"; +import { unsafeAssumeValidArg } from "@/runtime/runtimeTypes"; +import ConsoleLogger from "@/utils/ConsoleLogger"; +import { InputValidationError } from "@/blocks/errors"; +import { BusinessError } from "@/errors"; + +describe("parse compile error", () => { + test("invalid fromdate", async () => { + // https://github.com/pixiebrix/pixiebrix-extension/issues/3216 + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: '"" | fromdate', input: {} }), + { + ctxt: {}, + root: null, + logger: new ConsoleLogger(), + } + ); + + await expect(promise).rejects.toThrowError(InputValidationError); + await expect(promise).rejects.toThrowError( + 'date "" does not match format "%Y-%m-%dT%H:%M:%SZ"' + ); + }); + + test("missing brace", async () => { + // https://github.com/pixiebrix/pixiebrix-extension/issues/3216 + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: "{", input: {} }), + { + ctxt: {}, + root: null, + logger: new ConsoleLogger(), + } + ); + + await expect(promise).rejects.toThrowError(InputValidationError); + await expect(promise).rejects.toThrowError( + "Unexpected end of jq filter, are you missing a parentheses, brace, and/or quote mark?" + ); + }); + + test("null iteration", async () => { + // https://github.com/pixiebrix/pixiebrix-extension/issues/3216 + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: ".foo[]", input: {} }), + { + ctxt: {}, + root: null, + logger: new ConsoleLogger(), + } + ); + + await expect(promise).rejects.toThrowError(BusinessError); + await expect(promise).rejects.toThrowError( + "Cannot iterate over null (null)" + ); + }); +}); diff --git a/src/blocks/transformers/jq.ts b/src/blocks/transformers/jq.ts index a2064877db..0ee24c7fc8 100644 --- a/src/blocks/transformers/jq.ts +++ b/src/blocks/transformers/jq.ts @@ -20,7 +20,7 @@ import { BlockArg, BlockOptions, Schema } from "@/core"; import { propertiesToSchema } from "@/validators/generic"; import { isNullOrBlank } from "@/utils"; import { InputValidationError } from "@/blocks/errors"; -import { isErrorObject } from "@/errors"; +import { BusinessError, isErrorObject } from "@/errors"; const jqStacktraceRegexp = /jq: error \(at :0\): (?.*)/; @@ -74,6 +74,11 @@ export class JQTransformer extends Transformer { !isErrorObject(error) || (error.message.length > 13 && !error.message.includes("compile error")) ) { + // Unless there's bug in jq itself, if there's an error at this point, it's business error + if (isErrorObject(error)) { + throw new BusinessError(error.message, { cause: error }); + } + throw error; } diff --git a/src/errors.test.ts b/src/errors.test.ts index fdba227d04..9ad8afcf88 100644 --- a/src/errors.test.ts +++ b/src/errors.test.ts @@ -27,7 +27,7 @@ import { MultipleElementsFoundError, NoElementsFoundError, selectError, - serializePixiebrixError, + serializeErrorAndProperties, } from "@/errors"; import { range } from "lodash"; import { deserializeError, serializeError } from "serialize-error"; @@ -306,8 +306,10 @@ describe("serialization", () => { cause: inputValidationError, }); - const serializedError = serializePixiebrixError(contextError); + const serializedError = serializeErrorAndProperties(contextError); + // Use the isPlainObject from "@reduxjs/toolkit" because it's Redux that requires the object in the state to be + // serializable. We want it to be serializable and especially serializable for redux. expect(isPlainObject(serializedError)).toBeTruthy(); expect(isPlainObject(serializedError.cause)).toBeTruthy(); }); diff --git a/src/errors.ts b/src/errors.ts index 3369d89e2d..6c56804f79 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -250,11 +250,12 @@ export function isContextError(error: unknown): error is ContextError { ); } -// The serializeError preserves custom properties that effectively means the "cause" is skipped by the serializer -export function serializePixiebrixError(error: unknown): ErrorObject { - const serializedError = serializeError(error); +// `serialize-error/serializeError` preserves custom properties, so "cause" in our errors, e.g., ContextError is skipped +// https://github.com/sindresorhus/serialize-error/issues/74 +export function serializeErrorAndProperties(error: unknown): ErrorObject { + const serializedError = serializeError(error, { useToJSON: false }); if (typeof error === "object" && "cause" in error) { - serializedError.cause = serializePixiebrixError( + serializedError.cause = serializeErrorAndProperties( (error as { cause: unknown }).cause ); }