diff --git a/src/background/logging.ts b/src/background/logging.ts index 417da2627b..51f6436c8f 100644 --- a/src/background/logging.ts +++ b/src/background/logging.ts @@ -19,7 +19,7 @@ import { uuidv4 } from "@/types/helpers"; import { getRollbar } from "@/telemetry/initRollbar"; import { MessageContext, SerializedError, UUID } from "@/core"; import { Except, JsonObject } from "type-fest"; -import { deserializeError, serializeError } from "serialize-error"; +import { deserializeError } from "serialize-error"; import { DBSchema, openDB } from "idb/with-async-ittr"; import { isEmpty, once, sortBy } from "lodash"; import { allowsTrack } from "@/telemetry/dnt"; @@ -32,6 +32,7 @@ import { IGNORED_ERROR_PATTERNS, isAxiosError, isContextError, + serializeErrorAndProperties, } from "@/errors"; import { expectContext, forbidContext } from "@/utils/expectContext"; import { isAppRequest, selectAbsoluteUrl } from "@/services/requestErrorUtils"; @@ -331,9 +332,9 @@ export async function recordError( context: flatContext, message, data, - - // Ensure it's serialized - error: serializeError(maybeSerializedError), + // 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..2b514230d8 --- /dev/null +++ b/src/blocks/transformers/jq.test.ts @@ -0,0 +1,105 @@ +/* + * 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("smoke tests", () => { + test("passes input to filter", async () => { + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: ".foo", data: { foo: 42 } }), + { + ctxt: {}, + root: null, + logger: new ConsoleLogger(), + } + ); + + await expect(promise).resolves.toStrictEqual(42); + }); +}); + +describe("ctxt", () => { + test.each([[null], [""]])("pass context if data is %s", async (data) => { + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: ".foo", data }), + { + ctxt: { foo: 42 }, + root: null, + logger: new ConsoleLogger(), + } + ); + + await expect(promise).resolves.toStrictEqual(42); + }); +}); + +describe("parse compile error", () => { + test("invalid fromdate", async () => { + // https://github.com/pixiebrix/pixiebrix-extension/issues/3216 + const promise = new JQTransformer().transform( + unsafeAssumeValidArg({ filter: '"" | fromdate', data: {} }), + { + 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: "{", data: {} }), + { + 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[]", data: {} }), + { + 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 e720e8b745..0ee24c7fc8 100644 --- a/src/blocks/transformers/jq.ts +++ b/src/blocks/transformers/jq.ts @@ -20,7 +20,9 @@ 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\): (?.*)/; export class JQTransformer extends Transformer { override async isPure(): Promise { @@ -66,13 +68,24 @@ export class JQTransformer extends Transformer { // eslint-disable-next-line @typescript-eslint/return-await -- Type is `any`, it throws the rule off return await jq.promised.json(input, filter); } catch (error) { - if (!isErrorObject(error) || !error.message.includes("compile error")) { + // The message length check is there because the JQ error message sometimes is cut and if it is we try to parse the stacktrace + // See https://github.com/pixiebrix/pixiebrix-extension/issues/3216 + if ( + !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; } const message = error.stack.includes("unexpected $end") ? "Unexpected end of jq filter, are you missing a parentheses, brace, and/or quote mark?" - : "Invalid jq filter, see error log for details"; + : jqStacktraceRegexp.exec(error.stack)?.groups?.message?.trim() ?? + "Invalid jq filter, see error log for details"; throw new InputValidationError( message, diff --git a/src/errors.test.ts b/src/errors.test.ts index 50b0c06762..9ad8afcf88 100644 --- a/src/errors.test.ts +++ b/src/errors.test.ts @@ -27,11 +27,13 @@ import { MultipleElementsFoundError, NoElementsFoundError, selectError, + serializeErrorAndProperties, } from "@/errors"; import { range } from "lodash"; import { deserializeError, serializeError } from "serialize-error"; import { InputValidationError, OutputValidationError } from "@/blocks/errors"; import { matchesAnyPattern } from "@/utils"; +import { isPlainObject } from "@reduxjs/toolkit"; const TEST_MESSAGE = "Test message"; @@ -283,11 +285,32 @@ describe("selectError", () => { it("wraps primitive from PromiseRejectionEvent", () => { const errorEvent = new PromiseRejectionEvent( "error", - createUncaughtRejection("It’s a non-error") + createUncaughtRejection("It's a non-error") ); expect(selectError(errorEvent)).toMatchInlineSnapshot( - "[Error: It’s a non-error]" + "[Error: It's a non-error]" ); }); }); + +describe("serialization", () => { + test("serializes error cause", () => { + const inputValidationError = new InputValidationError( + "test input validation error", + null, + null, + [] + ); + const contextError = new ContextError("text context error", { + cause: inputValidationError, + }); + + 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 19e798d39d..6c56804f79 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -16,7 +16,7 @@ */ import { MessageContext } from "@/core"; -import { deserializeError, ErrorObject } from "serialize-error"; +import { deserializeError, ErrorObject, serializeError } from "serialize-error"; import { AxiosError, AxiosResponse } from "axios"; import { isObject, matchesAnyPattern } from "@/utils"; import safeJsonStringify from "json-stringify-safe"; @@ -250,6 +250,19 @@ export function isContextError(error: unknown): error is ContextError { ); } +// `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 = serializeErrorAndProperties( + (error as { cause: unknown }).cause + ); + } + + return serializedError; +} + /** * Returns true iff the root cause of the error was a CancelError. * @param error the error object