polish HTTP 304 docs/test/handling (#4129)

After #4120 was merged, here's a bit of polish on top.

- docs: Optimize JSDoc and comments.
- test: The existing 304 test is moved to the dedicated status code
block and extended with an `errorSpy` to also verify that no `error`
event is emitted.
- refactor: The success branch is moved before the error branch in
`fetch()`. This is more intuitive.

The bottom line is that this PR improves maintainability. For users,
this PR doesn't change anything.
This commit is contained in:
Kristjan ESPERANTO
2026-05-01 00:02:01 +02:00
committed by GitHub
parent 01ae1b443d
commit 0905d66a91
2 changed files with 35 additions and 32 deletions
+7 -7
View File
@@ -32,7 +32,7 @@ const ERROR_TYPE_TO_TRANSLATION = {
* - Authentication support (Basic, Bearer)
* - Self-signed certificate support
* @augments EventEmitter
* @fires HTTPFetcher#response - When fetch succeeds with ok response
* @fires HTTPFetcher#response - When fetch succeeds (including 304 Not Modified)
* @fires HTTPFetcher#error - When fetch fails or returns non-ok response
* @example
* const fetcher = new HTTPFetcher(url, { reloadInterval: 60000 });
@@ -295,21 +295,21 @@ class HTTPFetcher extends EventEmitter {
const isSuccessfulResponse = response.ok || response.status === 304;
if (!isSuccessfulResponse) {
const { delay, errorInfo } = this.#getDelayForResponse(response);
nextDelay = delay;
this.emit("error", errorInfo);
} else {
if (isSuccessfulResponse) {
// Reset error counts on success
this.serverErrorCount = 0;
this.networkErrorCount = 0;
/**
* Response event - fired when fetch succeeds
* Response event - fired when fetch succeeds (including 304)
* @event HTTPFetcher#response
* @type {Response}
*/
this.emit("response", response);
} else {
const { delay, errorInfo } = this.#getDelayForResponse(response);
nextDelay = delay;
this.emit("error", errorInfo);
}
} catch (error) {
const isTimeout = error.name === "AbortError";
+28 -25
View File
@@ -51,31 +51,6 @@ describe("HTTPFetcher", () => {
expect(text).toBe(responseData);
});
it("should treat 304 responses as successful and reset error counters", async () => {
server.use(
http.get(TEST_URL, () => {
return new HttpResponse(null, { status: 304 });
})
);
fetcher = new HTTPFetcher(TEST_URL, { reloadInterval: 60000 });
fetcher.serverErrorCount = 2;
fetcher.networkErrorCount = 3;
const responsePromise = new Promise((resolve) => {
fetcher.on("response", (response) => {
resolve(response);
});
});
fetcher.startPeriodicFetch();
const response = await responsePromise;
expect(response.status).toBe(304);
expect(fetcher.serverErrorCount).toBe(0);
expect(fetcher.networkErrorCount).toBe(0);
});
it("should emit error event on network failure", async () => {
server.use(
http.get(TEST_URL, () => {
@@ -126,6 +101,34 @@ describe("HTTPFetcher", () => {
});
describe("HTTPFetcher - HTTP status code handling", () => {
describe("304 Not Modified", () => {
it("should emit response event for 304 and not emit error", async () => {
server.use(
http.get(TEST_URL, () => {
return new HttpResponse(null, { status: 304 });
})
);
fetcher = new HTTPFetcher(TEST_URL, { reloadInterval: 60000 });
fetcher.serverErrorCount = 2;
fetcher.networkErrorCount = 3;
const responsePromise = new Promise((resolve) => {
fetcher.on("response", resolve);
});
const errorSpy = vi.fn();
fetcher.on("error", errorSpy);
fetcher.startPeriodicFetch();
const response = await responsePromise;
expect(response.status).toBe(304);
expect(errorSpy).not.toHaveBeenCalled();
expect(fetcher.serverErrorCount).toBe(0);
expect(fetcher.networkErrorCount).toBe(0);
});
});
describe("401/403 errors (Auth failures)", () => {
it("should emit error with AUTH_FAILURE for 401", async () => {
server.use(