Skip to content

Commit

Permalink
[IMP] compiler: improve error message when failing to compile template
Browse files Browse the repository at this point in the history
Previously, when a template failed to compile because of a syntax error
(typically because we don't do any syntax checking when compiling
expression, allowing invalid expressions to be transpiled successfully),
the error reporting was very minimal: you would only get the error
message itself (eg: "Unexpected token") with the stack information of
the error pointing to the call to `new Function` in owl, which is not
very useful.

This commit catches the compilation error and completes it with
information about the template name when available, and also adds the
generated code to the error message, allowing the user to just
copy/paste it in their web console or code editor to get a more precise
location for the error.
  • Loading branch information
sdegueldre authored and ged-odoo committed Jul 20, 2023
1 parent 7bc9d34 commit 9d99f89
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 2 deletions.
13 changes: 12 additions & 1 deletion src/compiler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { TemplateSet } from "../runtime/template_set";
import type { BDom } from "../runtime/blockdom";
import { CodeGenerator, Config } from "./code_generator";
import { parse } from "./parser";
import { OwlError } from "../runtime";

export type Template = (context: any, vnode: any, key?: string) => BDom;

Expand All @@ -27,5 +28,15 @@ export function compile(
const codeGenerator = new CodeGenerator(ast, { ...options, hasSafeContext });
const code = codeGenerator.generateCode();
// template function
return new Function("app, bdom, helpers", code) as TemplateFunction;
try {
return new Function("app, bdom, helpers", code) as TemplateFunction;
} catch (originalError: any) {
const { name } = options;
const nameStr = name ? `template "${name}"` : "anonymous template";
const err = new OwlError(
`Failed to compile ${nameStr}: ${originalError.message}\n\ngenerated code:\nfunction(app, bdom, helpers) {\n${code}\n}`
);
err.cause = originalError;
throw err;
}
}
18 changes: 18 additions & 0 deletions tests/compiler/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,22 @@ describe("basic validation", () => {
const template = `<div t-best-beer="rochefort 10">test</div>`;
expect(() => renderToString(template)).toThrow("Unknown QWeb directive: 't-best-beer'");
});

test("compilation error", () => {
const template = `<div t-att-class="a b">test</div>`;
expect(() => renderToString(template))
.toThrow(`Failed to compile anonymous template: Unexpected identifier
generated code:
function(app, bdom, helpers) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let block1 = createBlock(\`<div block-attribute-0="class">test</div>\`);
return function template(ctx, node, key = "") {
let attr1 = ctx['a']ctx['b'];
return block1([attr1]);
}
}`);
});
});
12 changes: 12 additions & 0 deletions tests/components/__snapshots__/error_handling.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ exports[`basics display a nice error if a component is not a component 1`] = `
}"
`;

exports[`basics display a nice error if a non-root component template fails to compile 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
const comp1 = app.createComponent(\`Child\`, true, false, false, []);

return function template(ctx, node, key = \\"\\") {
return comp1({}, key + \`__1\`, node, this, null);
}
}"
`;

exports[`basics display a nice error if it cannot find component (in dev mode) 1`] = `
"function anonymous(app, bdom, helpers
) {
Expand Down
60 changes: 60 additions & 0 deletions tests/components/error_handling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,66 @@ describe("basics", () => {
);
});

test("display a nice error if the root component template fails to compile", async () => {
// This is a special case: mount throws synchronously and we don't have any
// node which can handle the error, hence the different structure of this test
class Comp extends Component {
static template = xml`<div t-att-class="a b">test</div>`;
}
const app = new App(Comp);
let error: Error;
try {
await app.mount(fixture);
} catch (e) {
error = e as Error;
}
const expectedErrorMessage = `Failed to compile anonymous template: Unexpected identifier
generated code:
function(app, bdom, helpers) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let block1 = createBlock(\`<div block-attribute-0="class">test</div>\`);
return function template(ctx, node, key = "") {
let attr1 = ctx['a']ctx['b'];
return block1([attr1]);
}
}`;
expect(error!).toBeDefined();
expect(error!.message).toBe(expectedErrorMessage);
});

test("display a nice error if a non-root component template fails to compile", async () => {
class Child extends Component {
static template = xml`<div t-att-class="a b">test</div>`;
}
class Parent extends Component {
static components = { Child };
static template = xml`<Child/>`;
}
const expectedErrorMessage = `Failed to compile anonymous template: Unexpected identifier
generated code:
function(app, bdom, helpers) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let block1 = createBlock(\`<div block-attribute-0="class">test</div>\`);
return function template(ctx, node, key = "") {
let attr1 = ctx['a']ctx['b'];
return block1([attr1]);
}
}`;
const app = new App(Parent as typeof Component);
let error: Error;
const mountProm = app.mount(fixture).catch((e: Error) => (error = e));
await expect(nextAppError(app)).resolves.toThrow(expectedErrorMessage);
await mountProm;
expect(error!).toBeDefined();
expect(error!.message).toBe(expectedErrorMessage);
});

test("simple catchError", async () => {
class Boom extends Component {
static template = xml`<div t-esc="a.b.c"/>`;
Expand Down
2 changes: 1 addition & 1 deletion tests/misc/portal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ describe("Portal: Props validation", () => {
error = e as Error;
}
expect(error!).toBeDefined();
expect(error!.message).toBe(`Unexpected token ','`);
expect(error!.message).toContain(`Unexpected token ','`);
});

test("target must be a valid selector", async () => {
Expand Down

0 comments on commit 9d99f89

Please sign in to comment.