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

#6408: jq error handling improvements #6415

Merged
merged 18 commits into from
Sep 10, 2023
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
205 changes: 171 additions & 34 deletions src/bricks/transformers/jq.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { BusinessError } from "@/errors/businessErrors";
import { serializeError } from "serialize-error";
import { throwIfInvalidInput } from "@/runtime/runtimeUtils";
import { type RenderedArgs } from "@/types/runtimeTypes";
import { range } from "lodash";

describe("smoke tests", () => {
test("passes input to filter", async () => {
Expand All @@ -40,6 +41,34 @@ describe("smoke tests", () => {

await expect(promise).resolves.toStrictEqual(42);
});

test("can run concurrently", async () => {
// Smoke test we don't get `generic error, no stack` errors when running concurrently on node.
// Pick a number that's quick-enough to run on CI
const runCount = 20;

const brick = new JQTransformer();
const values = Promise.all(
range(runCount).map(async (number) =>
brick.transform(
unsafeAssumeValidArg({
filter: ".foo.data",
data: { foo: { data: number } },
}),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
)
)
);

// There shouldn't be any interference between the concurrent runs
await expect(values).resolves.toStrictEqual(range(runCount).map((n) => n));
});
});

describe("ctxt", () => {
Expand All @@ -59,19 +88,32 @@ describe("ctxt", () => {
});
});

describe("parse compile error", () => {
test("compile error has correct metadata", async () => {
describe("json", () => {
test("string data is not interpreted", async () => {
const promise = new JQTransformer().transform(
unsafeAssumeValidArg({ filter: ".", data: "[]" }),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);

// String is returned as-is, not as a JSON array
await expect(promise).resolves.toStrictEqual("[]");
});
});

describe("jq compilation errors", () => {
test("error metadata matches", async () => {
try {
await new JQTransformer().transform(
unsafeAssumeValidArg({ filter: '"" | fromdate', data: {} }),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);
await throwIfInvalidInput(new JQTransformer(), {
filter: 42,
data: {},
} as unknown as RenderedArgs);
expect.fail("Invalid test, expected validateInput to throw");
} catch (error) {
expect(serializeError(error)).toStrictEqual({
name: "InputValidationError",
Expand All @@ -80,24 +122,35 @@ describe("parse compile error", () => {
message: expect.toBeString(),
stack: expect.toBeString(),
errors: [
{
error: expect.toBeString(),
instanceLocation: "#",
keyword: "properties",
keywordLocation: "#/properties",
},
{
error: expect.toBeString(),
instanceLocation: "#/filter",
keyword: "format",
keywordLocation: "#/properties/filter/format",
keyword: "type",
keywordLocation: "#/properties/filter/type",
},
],
});
}
});

test("error metadata matches", async () => {
test("compile error has correct metadata", async () => {
try {
await throwIfInvalidInput(new JQTransformer(), {
filter: 42,
data: {},
} as unknown as RenderedArgs);
expect.fail("Invalid test, expected validateInput to throw");
await new JQTransformer().transform(
unsafeAssumeValidArg({ filter: '"', data: {} }),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);
} catch (error) {
expect(serializeError(error)).toStrictEqual({
name: "InputValidationError",
Expand All @@ -106,27 +159,21 @@ describe("parse compile error", () => {
message: expect.toBeString(),
stack: expect.toBeString(),
errors: [
{
error: expect.toBeString(),
instanceLocation: "#",
keyword: "properties",
keywordLocation: "#/properties",
},
{
error: expect.toBeString(),
instanceLocation: "#/filter",
keyword: "type",
keywordLocation: "#/properties/filter/type",
keyword: "format",
keywordLocation: "#/properties/filter/format",
},
],
});
}
});

test("invalid fromdate", async () => {
test("missing brace", async () => {
// https://github.com/pixiebrix/pixiebrix-extension/issues/3216
const promise = new JQTransformer().transform(
unsafeAssumeValidArg({ filter: '"" | fromdate', data: {} }),
unsafeAssumeValidArg({ filter: "{", data: {} }),
{
ctxt: {},
root: null,
Expand All @@ -138,14 +185,14 @@ describe("parse compile error", () => {

await expect(promise).rejects.toThrow(InputValidationError);
await expect(promise).rejects.toThrow(
'date "" does not match format "%Y-%m-%dT%H:%M:%SZ"'
"Unexpected end of jq filter, are you missing a parentheses, brace, and/or quote mark?"
);
});

test("missing brace", async () => {
test("multiple compile errors", async () => {
// https://github.com/pixiebrix/pixiebrix-extension/issues/3216
const promise = new JQTransformer().transform(
unsafeAssumeValidArg({ filter: "{", data: {} }),
unsafeAssumeValidArg({ filter: "a | b", data: {} }),
{
ctxt: {},
root: null,
Expand All @@ -157,10 +204,12 @@ describe("parse compile error", () => {

await expect(promise).rejects.toThrow(InputValidationError);
await expect(promise).rejects.toThrow(
"Unexpected end of jq filter, are you missing a parentheses, brace, and/or quote mark?"
"Invalid jq filter, see error log for details"
);
});
});

describe("jq execution errors", () => {
test("null iteration", async () => {
// https://github.com/pixiebrix/pixiebrix-extension/issues/3216
const promise = new JQTransformer().transform(
Expand All @@ -177,4 +226,92 @@ describe("parse compile error", () => {
await expect(promise).rejects.toThrow(BusinessError);
await expect(promise).rejects.toThrow("Cannot iterate over null (null)");
});

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(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);

await expect(promise).rejects.toThrow(BusinessError);
await expect(promise).rejects.toThrow(
'date "" does not match format "%Y-%m-%dT%H:%M:%SZ"'
);
});
});

describe("known jq-web bugs and quirks", () => {
test("Error if no result set produced", async () => {
// https://github.com/fiatjaf/jq-web/issues/32
const promise = new JQTransformer().transform(
unsafeAssumeValidArg({ filter: ".[] | .Title", data: [] }),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);

await expect(promise).rejects.toThrow(BusinessError);
await expect(promise).rejects.toThrow(
"ensure the jq filter produces a result for the data"
);
});

test.skip("running 2048+ times causes FS errors", async () => {
// https://github.com/fiatjaf/jq-web/issues/18
// Skipping on CI because it's too slow to run this test

const brick = new JQTransformer();
const values = Promise.all(
range(3000).map(async (number) =>
brick.transform(
unsafeAssumeValidArg({
filter: ".foo.data",
data: { foo: { data: number } },
}),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
)
)
);

await expect(values).rejects.toThrow(
"Error opening stream, reload the page"
);

// Uncomment this when the bug in the dependency has been fixed
// await expect(values).resolves.toStrictEqual(range(3000).map((n) => n));
});

test("error using modulo operator in filter", async () => {
// https://github.com/fiatjaf/jq-web/issues/19
const promise = new JQTransformer().transform(
unsafeAssumeValidArg({ filter: "1 % 1", data: [] }),
{
ctxt: {},
root: null,
logger: new ConsoleLogger(),
runPipeline: neverPromise,
runRendererPipeline: neverPromise,
}
);

await expect(promise).rejects.toThrow(BusinessError);
await expect(promise).rejects.toThrow("wA is not a function");
});
});
Loading