diff --git a/.changeset/client-error-action.md b/.changeset/client-error-action.md new file mode 100644 index 000000000..5eb0ae82a --- /dev/null +++ b/.changeset/client-error-action.md @@ -0,0 +1,11 @@ +--- +"webpack-dev-middleware": minor +--- + +The runtime understands `{ action: "error", message }`, for something a server decided about one client rather than about a build. + +A server applying a policy of its own — webpack-dev-server's `allowedHosts` is the case this exists for — refuses a client and holds the only explanation for it. There was nowhere to put that: an unknown action reaches a `subscribeAll` handler and otherwise goes nowhere, so the connection closed with the reason nowhere the developer was looking. The message is now logged as the server gave it, and posted to the page as `webpackError` so tooling watching the stream sees it too. + +`publishTo(client, payload)` is on the instance alongside `publish`, since that is how the message reaches the one client being refused. Writing to the client directly means knowing which transport is carrying it — a WebSocket client and the event stream's `ServerResponse` are not the same kind of object, and neither one's write method exists on the other, so a message sent the wrong way silently never arrives. + +It carries no policy with it. Which clients to refuse, and why, stays entirely the server's; this is only somewhere to say so. diff --git a/README.md b/README.md index d190b8a56..32167e518 100644 --- a/README.md +++ b/README.md @@ -1235,6 +1235,39 @@ chokidar.watch("content/**/*.md").on("change", () => { }); ``` +`{ action: "error", message }` is for something a server decided about one +client rather than about a build — refused for where it connected from, say, +by a policy the server applies and this middleware does not. The runtime logs +the message as given and posts it to the page as `webpackError`. The server +has the only explanation; without somewhere to put it, the connection closes +with the reason nowhere the developer is looking: + +```js +instance.onConnect((client, req) => { + if (!allowed(req.headers.origin)) { + instance.publishTo(client, { + action: "error", + message: `Origin "${req.headers.origin}" is not allowed.`, + }); + + // The two transports hand over different kinds of client: a WebSocket, + // which closes, and the event stream's `ServerResponse`, which ends. + // `publishTo` above does not need telling apart; dropping the connection + // does. + if (typeof client.close === "function") { + client.close(); + } else { + client.end(); + } + } +}); +``` + +[`publishTo`](#publishtoclient-payload) is how the message reaches that one +client whichever transport is carrying it — writing to the client directly +means knowing which kind it is, and a message written to the wrong one simply +never arrives. + Nothing is sent when no client is connected, so a caller does not have to ask whether anyone is listening — but that is the only traffic it saves, and with a page open every call is a message. Keeping a chatty source down to what changed, as above, is the caller's. Does nothing when `hot` is disabled. #### Parameters @@ -1244,7 +1277,50 @@ Nothing is sent when no client is connected, so a caller does not have to ask wh Type: `{ action: String, ...}` Required: `Yes` -An `action` the clients understand, and whatever that action carries. The built-in actions are `building`, `progress`, `built`, `sync` and `reload`. +An `action` the clients understand, and whatever that action carries. The built-in actions are `building`, `progress`, `built`, `sync`, `reload` and `error`. + +### `publishTo(client, payload)` + +The same, to one client rather than every one: what a server answering a +single connection needs — refusing it, most of all, which is the only thing +that connection is owed an explanation for. + +```js +instance.onConnect((client, req) => { + if (!allowed(req.headers.origin)) { + instance.publishTo(client, { + action: "error", + message: `Origin "${req.headers.origin}" is not allowed.`, + }); + } +}); +``` + +The client is one [`onConnect`](#onconnectfn) handed over. Writing to it +directly instead means knowing which transport is carrying it — a WebSocket +client and the event stream's `ServerResponse` are not the same kind of +object, and neither one's write method exists on the other — so a message sent +the wrong way silently never arrives. This takes a client of either and puts +the payload on the wire that client is actually on. + +Declines to write to a client that is no longer open, and does nothing when +`hot` is disabled. + +#### Parameters + +##### `client` + +Type: `Object` +Required: `Yes` + +A client [`onConnect`](#onconnectfn) was called with. + +##### `payload` + +Type: `{ action: String, ...}` +Required: `Yes` + +As [`publish`](#publishpayload). ### `close(callback)` diff --git a/client-src/index.js b/client-src/index.js index 0725d5f09..9d1d63f48 100644 --- a/client-src/index.js +++ b/client-src/index.js @@ -646,6 +646,17 @@ function processMessage(obj) { sendMessage("Invalid"); break; } + case "error": { + // Something the server decided about this client, rather than about a + // build: refused for where it connected from, turned away by a policy + // the server applies and this middleware does not. The server knows why + // and the page does not, so what it says is logged as it was given — + // the alternative is a connection that closes with no explanation + // anywhere the developer is looking. + log.error(obj.message || "The server refused the connection."); + sendMessage("Error", obj.message); + break; + } case "reload": { // The server asking for the page outright, for a change no compilation // knows about — a file served from disk, say. Not a build, so `hot` and diff --git a/src/hot.js b/src/hot.js index 5399849be..663a4655e 100644 --- a/src/hot.js +++ b/src/hot.js @@ -465,6 +465,7 @@ function publishBundles(bundles, previousBundles, eventStream) { * @property {(fn: (client: EXPECTED_ANY, req: IncomingMessage) => void) => void} onConnect called with each client once it has joined, and the request it joined with, before anything is published to it * @property {(req: IncomingMessage, res: ServerResponse) => void} handle answer a request on the endpoint's path * @property {(payload: Payload | CustomPayload) => void} publish publish a payload to every client + * @property {(client: EXPECTED_ANY, payload: Payload | CustomPayload) => void} publishTo publish a payload to one client, for answering a single connection * @property {() => void} close end every client and detach the heartbeat */ @@ -699,6 +700,17 @@ function createHot(compiler, userOptions, statsOption) { eventStream.publish(payload); }, + publishTo(client, payload) { + if (closed) return; + + // No `hasClients` guard: the caller named the client, so it knows + // someone is listening. This is for answering one connection, which a + // server refusing it has to be able to do without knowing which + // transport is carrying it — a WebSocket client and an event-stream + // client are not the same kind of object. A transport declines to write + // to a client that is no longer open. + eventStream.publishTo(client, payload); + }, close() { if (closed) return; diff --git a/src/index.js b/src/index.js index 582dcf92b..19a5376b5 100644 --- a/src/index.js +++ b/src/index.js @@ -198,6 +198,12 @@ const noop = () => {}; * @param {import("./hot").Payload | import("./hot").CustomPayload} payload the payload to publish to every client */ +/** + * @callback PublishTo + * @param {EXPECTED_ANY} client a client `onConnect` handed over + * @param {import("./hot").Payload | import("./hot").CustomPayload} payload the payload to publish to it + */ + /** * @callback OnConnect * @param {(client: EXPECTED_ANY, req: IncomingMessage) => void} fn called with each client once it has joined, and the request it joined with, before anything is published to it @@ -219,6 +225,7 @@ const noop = () => {}; * @property {HandleUpgrade} handleUpgrade answer one WebSocket upgrade, for a server that owns its own `upgrade` event * @property {OnConnect} onConnect called with each client that joins, and the request it joined with * @property {Publish} publish put a payload of your own on the hot stream, for what a server measures itself — a no-op when `hot` is off + * @property {PublishTo} publishTo put a payload on the hot stream for one client, for answering a single connection — whichever transport is carrying it, and a no-op when `hot` is off * @property {(string | false | undefined)=} token the secret the hot endpoint requires, for a client of your own to put on the url; false when it requires none, undefined when `hot` is off * @property {Close} close close * @property {Context} context context @@ -802,6 +809,18 @@ function wdm(compiler, options = {}, isPlugin = false) { } }; + // The same, to one client: what a server refusing a connection needs to say + // why, without having to know which transport is carrying it. A WebSocket + // client and an event-stream client are not the same kind of object, and + // writing to the wrong one is a message that silently never arrives. + // + // A no-op when `hot` is off, like `publish`. + instance.publishTo = (client, payload) => { + if (filledContext.hot) { + filledContext.hot.publishTo(client, payload); + } + }; + // The secret the hot endpoint requires, for a client of your own: the // injected one is handed it through its entry query, but anything you wrote // yourself has to put it on the url. `false` when the endpoint requires diff --git a/test/e2e/messages.test.js b/test/e2e/messages.test.js index 9b8b81a69..db0ed586b 100644 --- a/test/e2e/messages.test.js +++ b/test/e2e/messages.test.js @@ -1,3 +1,4 @@ +import collectConsole from "../helpers/console-collector"; import { acceptedApp, boomApp, @@ -113,6 +114,40 @@ describe("messages posted to the page (browser)", () => { expect(errors.join("\n")).toContain("Module parse failed"); }); + // A server applying a policy of its own — webpack-dev-server's + // `allowedHosts` is the one this exists for — refuses a client and has the + // only explanation for it. Without somewhere to put that, the connection + // closes with the reason nowhere the developer is looking. + it("logs what the server said when it refused a client", async () => { + hotApp = await createHotApp({ code: recordingApp("v1") }); + ({ page, browser } = await runBrowser()); + const console_ = collectConsole(page); + + await page.goto(hotApp.url); + await waitForAppText(page, "v1"); + await console_.waitFor("connected"); + + hotApp.instance.publish({ + action: "error", + message: "Invalid Host/Origin header", + }); + + await console_.waitFor("Invalid Host/Origin header"); + + expect(console_.messages.join("\n")).toContain( + "Invalid Host/Origin header", + ); + + // Posted to the page as well, so tooling watching the stream sees it too. + await page.waitForFunction( + () => + (globalThis.posted || []).some( + (message) => message && message.type === "webpackError", + ), + { timeout: 30000 }, + ); + }); + it("says when the connection went away, once per outage", async () => { hotApp = await createHotApp({ query: "?timeout=1000", diff --git a/test/instance-hot-api.test.js b/test/instance-hot-api.test.js index 9699a2289..2d5565383 100644 --- a/test/instance-hot-api.test.js +++ b/test/instance-hot-api.test.js @@ -107,6 +107,26 @@ describe("the hot API on the middleware instance", () => { expect(publish).toHaveBeenCalledWith(payload); }); + + // Answering one connection: a server refusing a client has to be able to + // say why, and it cannot do that by writing to the client itself — a + // WebSocket client and an event-stream client are not the same kind of + // object, and a `ServerResponse` has no `send`, so the message would + // silently never arrive. + it("publishes to a single client", () => { + const instance = build({ hot: true }); + // Mocked rather than spied through: the real one writes to the client, + // and this one is a stand-in rather than a connection. + const publishTo = jest + .spyOn(instance.context.hot, "publishTo") + .mockImplementation(() => {}); + const client = { marker: "one client" }; + const payload = { action: "error", message: "not allowed" }; + + instance.publishTo(client, payload); + + expect(publishTo).toHaveBeenCalledWith(client, payload); + }); }); describe("with hot disabled", () => { @@ -122,6 +142,14 @@ describe("the hot API on the middleware instance", () => { expect(() => instance.onConnect(() => {})).not.toThrow(); }); + it("takes a payload for one client and drops that too", () => { + const instance = build(); + + expect(() => + instance.publishTo({}, { action: "error", message: "x" }), + ).not.toThrow(); + }); + it("takes a payload and drops it, rather than making the caller ask", () => { const instance = build(); diff --git a/types/hot.d.ts b/types/hot.d.ts index 9b64cd301..b132248b8 100644 --- a/types/hot.d.ts +++ b/types/hot.d.ts @@ -9,6 +9,7 @@ export = createHot; * @property {(fn: (client: EXPECTED_ANY, req: IncomingMessage) => void) => void} onConnect called with each client once it has joined, and the request it joined with, before anything is published to it * @property {(req: IncomingMessage, res: ServerResponse) => void} handle answer a request on the endpoint's path * @property {(payload: Payload | CustomPayload) => void} publish publish a payload to every client + * @property {(client: EXPECTED_ANY, payload: Payload | CustomPayload) => void} publishTo publish a payload to one client, for answering a single connection * @property {() => void} close end every client and detach the heartbeat */ /** @@ -143,6 +144,10 @@ type HotInstance = { * publish a payload to every client */ publish: (payload: Payload | CustomPayload) => void; + /** + * publish a payload to one client, for answering a single connection + */ + publishTo: (client: EXPECTED_ANY, payload: Payload | CustomPayload) => void; /** * end every client and detach the heartbeat */ diff --git a/types/index.d.ts b/types/index.d.ts index e14dfdfae..d10be1ad6 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -60,6 +60,7 @@ declare namespace wdm { Attach, HandleUpgrade, Publish, + PublishTo, OnConnect, Close, AdditionalMethods, @@ -424,6 +425,10 @@ type HandleUpgrade = ( type Publish = ( payload: import("./hot").Payload | import("./hot").CustomPayload, ) => any; +type PublishTo = ( + client: EXPECTED_ANY, + payload: import("./hot").Payload | import("./hot").CustomPayload, +) => any; type OnConnect = ( fn: (client: EXPECTED_ANY, req: IncomingMessage) => void, ) => any; @@ -460,6 +465,10 @@ type AdditionalMethods< * put a payload of your own on the hot stream, for what a server measures itself — a no-op when `hot` is off */ publish: Publish; + /** + * put a payload on the hot stream for one client, for answering a single connection — whichever transport is carrying it, and a no-op when `hot` is off + */ + publishTo: PublishTo; /** * the secret the hot endpoint requires, for a client of your own to put on the url; false when it requires none, undefined when `hot` is off */