Skip to content

Commit

Permalink
#6408: jq error handling improvements (#6415)
Browse files Browse the repository at this point in the history
  • Loading branch information
@twschiller
twschiller authored Sep 10, 2023
1 parent e237877 commit 74c52f2
Show file tree
Hide file tree
Showing 4 changed files with 379 additions and 69 deletions.
205 changes: 171 additions & 34 deletions src/bricks/transformers/jq.test.ts
View file
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

0 comments on commit 74c52f2

Please sign in to comment.