diff --git a/packages/server/lib/serve/Supervisor.js b/packages/server/lib/serve/Supervisor.js index 091b7db0285..642f110bafb 100644 --- a/packages/server/lib/serve/Supervisor.js +++ b/packages/server/lib/serve/Supervisor.js @@ -546,10 +546,9 @@ class Supervisor extends EventEmitter { * Stops the server: closes live-reload, the HTTP socket, and the current BuildServer. Teardown * is tolerant: the socket is closed even if the BuildServer's destroy rejects. * - * @param {Function} [callback] Invoked once the HTTP server has closed * @returns {Promise} Resolves once teardown completes */ - async destroy(callback) { + async destroy() { // Move to the terminal state synchronously, before the first await, so an in-flight #swap or a // late definitionChanged sees DESTROYED at its next guard and adopts nothing. this.#setState(STATE.DESTROYED); @@ -560,8 +559,14 @@ class Supervisor extends EventEmitter { this.#definitionWatcher = null; this.#liveReloadHandle?.close(); this.#detachRelay(); - this.#httpServer?.close(callback); this.#clearRecoveryTimer(); + const httpClosed = new Promise((resolve) => { + if (!this.#httpServer) { + resolve(); + return; + } + this.#httpServer.close(() => resolve()); + }); try { await definitionWatcher?.destroy(); } catch (err) { @@ -572,6 +577,7 @@ class Supervisor extends EventEmitter { } catch (err) { log.verbose(`Error while destroying BuildServer: ${err?.message ?? err}`); } + await httpClosed; } } diff --git a/packages/server/lib/server.js b/packages/server/lib/server.js index ae434d484c4..6221a6789c6 100644 --- a/packages/server/lib/server.js +++ b/packages/server/lib/server.js @@ -20,6 +20,36 @@ const log = getLogger("server"); * @property {string[]} [ignorePaths=["test-resources/sap/ui/qunit/testrunner.html"]] */ +/** + * Stops a running server. + * + * Can be awaited or used with a callback. Called without arguments, it returns a + * Promise that resolves once teardown completes and rejects if teardown threw. + * Called with a callback, it returns undefined and invokes the callback once + * teardown completes, with no arguments on success or with the error as its first argument + * if teardown threw. + * + * @public + * @callback module:@ui5/server~closeServer + * @param {Function} [callback] Invoked once teardown completes. Receives the teardown error as + * its first argument if teardown threw, otherwise no arguments. + * @returns {Promise|undefined} A Promise that resolves once teardown completes + * when called without a callback, otherwise undefined. + */ + +/** + * Handle of a running server instance. + * + * @public + * @typedef {object} module:@ui5/server~ServerInstance + * @property {number} port Port the server is listening on + * @property {boolean} h2 Whether HTTP/2 is used + * @property {module:@ui5/server~closeServer} close Stops the server + * @property {Function} reinitialize Re-creates the serving stack. Returns a Promise + * that resolves once the new stack is in place. A no-op when no + * graphFactory was provided to {@link module:@ui5/server.serve}. + */ + /** * Start a server for the given project (sub-)tree. @@ -67,10 +97,7 @@ const log = getLogger("server"); * interface and does not depend on @ui5/project, so the owner (the UI5 CLI) * threads this in to provide the live re-resolution capability. Required * alongside graphFactory; omit both for a static serve. - * @returns {Promise} Promise resolving once the server is listening. - * It resolves with an object containing the port, - * h2-flag, a close function to stop the server, - * and a reinitialize function to re-create the serving stack. + * @returns {Promise} Promise resolving once the server is listening */ export async function serve(graph, { port, changePortIfInUse = false, h2 = false, key, cert, @@ -108,7 +135,12 @@ export async function serve(graph, { h2, port: supervisor.getPort(), close: function(callback) { - supervisor.destroy(callback); + const p = supervisor.destroy(); + if (callback) { + p.then(callback, callback); + } else { + return p; + } }, reinitialize: function() { return supervisor.reinitialize(); diff --git a/packages/server/test/lib/server/serve/Supervisor.js b/packages/server/test/lib/server/serve/Supervisor.js index a6735f3bdff..55b9dbe70d9 100644 --- a/packages/server/test/lib/server/serve/Supervisor.js +++ b/packages/server/test/lib/server/serve/Supervisor.js @@ -397,7 +397,7 @@ test("destroy() closes live-reload, the socket, and the BuildServer; reinitializ const supervisor = await Supervisor.create({}, baseConfig, undefined, graphFactory); - await new Promise((resolve) => supervisor.destroy(resolve)); + await supervisor.destroy(); t.true(liveReloadHandle.close.calledOnce); t.true(httpServer.close.calledOnce); @@ -415,7 +415,7 @@ test("destroy() closes the socket even when BuildServer.destroy() rejects", asyn const supervisor = await Supervisor.create({}, baseConfig, undefined, undefined); - await new Promise((resolve) => supervisor.destroy(resolve)); + await supervisor.destroy(); t.true(httpServer.close.calledOnce, "socket is closed despite the BuildServer destroy rejection"); }); @@ -1063,6 +1063,6 @@ test("destroy() tears the definition watcher down", async (t) => { const supervisor = await Supervisor.create({}, baseConfig, undefined, graphFactory); - await new Promise((resolve) => supervisor.destroy(resolve)); + await supervisor.destroy(); t.true(definitionWatchers[0].destroy.calledOnce, "watcher destroyed on teardown"); }); diff --git a/packages/server/test/lib/server/server.js b/packages/server/test/lib/server/server.js index e2cf9f9eeab..8eb49b5f505 100644 --- a/packages/server/test/lib/server/server.js +++ b/packages/server/test/lib/server/server.js @@ -10,7 +10,7 @@ import esmock from "esmock"; function createSupervisorMock({port = 3000, createRejects = null} = {}) { const supervisor = { getPort: sinon.stub().returns(port), - destroy: sinon.stub().callsFake((cb) => cb && cb()), + destroy: sinon.stub().resolves(), reinitialize: sinon.stub().resolves(), }; const create = createRejects ? @@ -78,6 +78,40 @@ test("serve() close() forwards to supervisor.destroy()", async (t) => { t.true(supervisor.destroy.calledOnce); }); +test("serve() close() passes the error to the callback when destroy rejects", async (t) => { + const {supervisor, Supervisor} = createSupervisorMock(); + const destroyError = new Error("teardown failed"); + supervisor.destroy = sinon.stub().rejects(destroyError); + const {serve} = await importServe(Supervisor); + + const result = await serve({}, {port: 3000}, undefined); + const err = await new Promise((resolve) => result.close(resolve)); + + t.is(err, destroyError, "the destroy rejection is forwarded to the close callback"); +}); + +test("serve() close() returns the destroy promise when no callback is passed", async (t) => { + const {supervisor, Supervisor} = createSupervisorMock(); + const {serve} = await importServe(Supervisor); + + const result = await serve({}, {port: 3000}, undefined); + await result.close(); + + t.true(supervisor.destroy.calledOnce); +}); + +test("serve() close() rejects the returned promise when destroy rejects", async (t) => { + const {supervisor, Supervisor} = createSupervisorMock(); + const destroyError = new Error("teardown failed"); + supervisor.destroy = sinon.stub().rejects(destroyError); + const {serve} = await importServe(Supervisor); + + const result = await serve({}, {port: 3000}, undefined); + const err = await t.throwsAsync(result.close()); + + t.is(err, destroyError, "the destroy rejection surfaces on the returned promise"); +}); + test("serve() rejects when Supervisor.create rejects", async (t) => { const createError = new Error("bind failed"); const {Supervisor} = createSupervisorMock({createRejects: createError});