Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/client-error-action.md
Original file line number Diff line number Diff line change
@@ -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.
78 changes: 77 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)`

Expand Down
11 changes: 11 additions & 0 deletions client-src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/hot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

Expand Down Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<RequestInternal, ResponseInternal>} context context
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions test/e2e/messages.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import collectConsole from "../helpers/console-collector";
import {
acceptedApp,
boomApp,
Expand Down Expand Up @@ -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",
Expand Down
28 changes: 28 additions & 0 deletions test/instance-hot-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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();

Expand Down
5 changes: 5 additions & 0 deletions types/hot.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
/**
Expand Down Expand Up @@ -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
*/
Expand Down
9 changes: 9 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ declare namespace wdm {
Attach,
HandleUpgrade,
Publish,
PublishTo,
OnConnect,
Close,
AdditionalMethods,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down
Loading