From 40fbbe2b521b07ccd4e0705720eeee326ad676f1 Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Fri, 4 Jun 2021 06:08:57 +0000 Subject: [PATCH] [IMP] web: make startServices promise reject when missing dependencies Previously, the promise returned by startServices was resolved when some services were missing dependencies. The rationale was that those dependencies could be fulfilled by bundles that might be in the page later. This rationale ignored the fact that bundles or scripts that are in the same page will be executed in the same microtick. This commit changes the returned promise from startServices such that it will throw if after all services that were registered in the registry at call time that could be started are started, and there are still services registered that could not be started for lack of dependencies. --- addons/web/static/src/env.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/addons/web/static/src/env.js b/addons/web/static/src/env.js index f5fbdc4f508..5578984c354 100644 --- a/addons/web/static/src/env.js +++ b/addons/web/static/src/env.js @@ -57,8 +57,11 @@ export const SERVICES_METADATA = {}; */ export async function startServices(env) { const toStart = new Set(); - let timeoutId; serviceRegistry.on("UPDATE", null, async (payload) => { + // Wait for all synchronous code so that if new services that depend on + // one another are added to the registry, they're all present before we + // start them. + await Promise.resolve(); const { operation, key: name, value: service } = payload; if (operation === "delete") { // We hardly see why it would be usefull to remove a service. @@ -70,10 +73,10 @@ export async function startServices(env) { const namedService = Object.assign(Object.create(service), { name }); toStart.add(namedService); } else { - timeoutId = await _startServices(env, toStart, timeoutId); + await _startServices(env, toStart); } }); - timeoutId = await _startServices(env, toStart, timeoutId); + await _startServices(env, toStart); } async function _startServices(env, toStart, timeoutId) { @@ -128,17 +131,17 @@ async function _startServices(env, toStart, timeoutId) { } } await start(); - clearTimeout(timeoutId); - timeoutId = undefined; if (toStart.size) { const names = [...toStart].map((s) => s.name); - timeoutId = setTimeout(() => { - timeoutId = undefined; - throw new Error(`Some services could not be started: ${names}`); - }, 15000); - toStart.clear(); + const missingDeps = new Set(); + [...toStart].forEach((s) => s.dependencies.forEach((dep) => { + if (!(dep in services) && !names.includes(dep)) { + missingDeps.add(dep); + } + })); + const depNames = [...missingDeps].join(", "); + throw new Error(`Some services could not be started: ${names}. Missing dependencies: ${depNames}`); } - return timeoutId; function findNext() { for (let s of toStart) {