Skip to content

Commit

Permalink
#3216 JQ fromdate error gets cut off in page editor (#3336)
Browse files Browse the repository at this point in the history
Co-authored-by: Todd Schiller <[email protected]>
  • Loading branch information
BALEHOK and twschiller authored May 6, 2022
1 parent 9a3c2b9 commit 74b411e
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 10 deletions.
9 changes: 5 additions & 4 deletions src/background/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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) {
Expand Down
105 changes: 105 additions & 0 deletions src/blocks/transformers/jq.test.ts
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*/

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)"
);
});
});
19 changes: 16 additions & 3 deletions src/blocks/transformers/jq.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <stdin>:0\): (?<message>.*)/;

export class JQTransformer extends Transformer {
override async isPure(): Promise<boolean> {
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 25 additions & 2 deletions src/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -283,11 +285,32 @@ describe("selectError", () => {
it("wraps primitive from PromiseRejectionEvent", () => {
const errorEvent = new PromiseRejectionEvent(
"error",
createUncaughtRejection("Its a non-error")
createUncaughtRejection("It's a non-error")
);

expect(selectError(errorEvent)).toMatchInlineSnapshot(
"[Error: Its 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();
});
});
15 changes: 14 additions & 1 deletion src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 74b411e

Please sign in to comment.