Skip to content

Commit

Permalink
[FIX] runtime: log if willStart takes more than 3s
Browse files Browse the repository at this point in the history
Before this commit, when willStart/willUpdateProps took more than
3s, a console.warn was done. In odoo, when a warning is logged
during a test, the test fails and the build is considered as "in
error".

There's a component that loads several resources (sequentially) in
its onWillStart, which *sometimes* takes more than 3s, making
builds fail non deterministically. Since a recent change (which
adds another call in the problematic onWillStart), the warning gets
logged quite often.

A quick fix is necessary, so we change the warn into a log, which
won't make build fail.

We may consider alternatives in the future though:
 - add a parameter to onWillStart, to disable the timeout, or to
   specify the delay (which is 3s by default)
 - do not warn in test mode
  • Loading branch information
aab-odoo committed Aug 14, 2024
1 parent f8bb868 commit 683a1f7
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion src/runtime/lifecycle_hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function wrapError(fn: (...args: any[]) => any, hookName: string) {
new Promise((resolve) => setTimeout(() => resolve(TIMEOUT), 3000)),
]).then((res) => {
if (res === TIMEOUT && node.fiber === fiber && node.status <= 2) {
console.warn(timeoutError);
console.log(timeoutError);
}
});
}
Expand Down
8 changes: 4 additions & 4 deletions tests/components/__snapshots__/lifecycle.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ exports[`lifecycle hooks sub widget (inside sub node): hooks are correctly calle
}"
`;

exports[`lifecycle hooks timeout in onWillStart doesn't emit a warning if app is destroyed 1`] = `
exports[`lifecycle hooks timeout in onWillStart doesn't emit a console log if app is destroyed 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
Expand All @@ -696,7 +696,7 @@ exports[`lifecycle hooks timeout in onWillStart doesn't emit a warning if app is
}"
`;

exports[`lifecycle hooks timeout in onWillStart emits a warning 1`] = `
exports[`lifecycle hooks timeout in onWillStart emits a console log 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
Expand All @@ -709,7 +709,7 @@ exports[`lifecycle hooks timeout in onWillStart emits a warning 1`] = `
}"
`;

exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 1`] = `
exports[`lifecycle hooks timeout in onWillUpdateProps emits a console log 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
Expand All @@ -723,7 +723,7 @@ exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 1`] = `
}"
`;

exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 2`] = `
exports[`lifecycle hooks timeout in onWillUpdateProps emits a console log 2`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
Expand Down
38 changes: 19 additions & 19 deletions tests/components/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ describe("lifecycle hooks", () => {
await mount(Test, fixture);
});

test("timeout in onWillStart emits a warning", async () => {
const { warn } = console;
let warnArgs: any[];
console.warn = jest.fn((...args) => (warnArgs = args));
test("timeout in onWillStart emits a console log", async () => {
const { log } = console;
let logArgs: any[];
console.log = jest.fn((...args) => (logArgs = args));
const { setTimeout } = window;
let timeoutCbs: any = {};
let timeoutId = 0;
Expand All @@ -138,17 +138,17 @@ describe("lifecycle hooks", () => {
}
await nextMicroTick();
await nextMicroTick();
expect(console.warn).toHaveBeenCalledTimes(1);
expect(warnArgs![0]!.message).toBe("onWillStart's promise hasn't resolved after 3 seconds");
expect(console.log).toHaveBeenCalledTimes(1);
expect(logArgs![0]!.message).toBe("onWillStart's promise hasn't resolved after 3 seconds");
} finally {
console.warn = warn;
console.log = log;
window.setTimeout = setTimeout;
}
});

test("timeout in onWillStart doesn't emit a warning if app is destroyed", async () => {
const { warn } = console;
console.warn = jest.fn();
test("timeout in onWillStart doesn't emit a console log if app is destroyed", async () => {
const { log } = console;
console.log = jest.fn();
const { setTimeout } = window;
let timeoutCbs: any = {};
let timeoutId = 0;
Expand All @@ -172,14 +172,14 @@ describe("lifecycle hooks", () => {
}
await nextMicroTick();
await nextMicroTick();
expect(console.warn).toHaveBeenCalledTimes(0);
expect(console.log).toHaveBeenCalledTimes(0);
} finally {
console.warn = warn;
console.log = log;
window.setTimeout = setTimeout;
}
});

test("timeout in onWillUpdateProps emits a warning", async () => {
test("timeout in onWillUpdateProps emits a console log", async () => {
class Child extends Component {
static template = xml``;
setup() {
Expand All @@ -193,9 +193,9 @@ describe("lifecycle hooks", () => {
}
const parent = await mount(Parent, fixture, { test: true });

const { warn } = console;
let warnArgs: any[];
console.warn = jest.fn((...args) => (warnArgs = args));
const { log } = console;
let logArgs: any[];
console.log = jest.fn((...args) => (logArgs = args));
const { setTimeout } = window;
let timeoutCbs: any = {};
let timeoutId = 0;
Expand All @@ -218,12 +218,12 @@ describe("lifecycle hooks", () => {
delete timeoutCbs[id];
}
await tick;
expect(console.warn).toHaveBeenCalledTimes(1);
expect(warnArgs![0]!.message).toBe(
expect(console.log).toHaveBeenCalledTimes(1);
expect(logArgs![0]!.message).toBe(
"onWillUpdateProps's promise hasn't resolved after 3 seconds"
);
} finally {
console.warn = warn;
console.log = log;
window.setTimeout = setTimeout;
}
});
Expand Down

0 comments on commit 683a1f7

Please sign in to comment.