klaemo opened a new pull request, #376: URL: https://github.com/apache/couchdb-nano/pull/376
## Overview If the connection breaks off after the response headers, reading the body fails. nano 11 currently swallows that failure. JSON responses resolve with `''`. For text, multipart and binary responses, the error escapes the un-awaited `responseHandler`, so the request promise never settles and Node reports an unhandled rejection. See #375 for details and a reproduction. This PR makes two changes to `lib/nano.js`: - **JSON bodies:** read with `response.text()`, then `JSON.parse()`. Only the parse step is inside the try/catch. A complete body that isn't valid JSON still resolves `''` as before, and a failed read now propagates. This keeps the "ignore unparseable bodies" behaviour without inspecting error types (`SyntaxError` vs `TypeError`). - **`relax()`:** the `.then()` callback now returns the `responseHandler(…)` promise, so the existing `.catch()` handles any failure from it. A body-read failure then rejects the same way a connection failure before the headers already does: `error happened in your connection. Reason: SocketError: other side closed`. The promise can't settle twice: `responseHandler` calls `resolve` as its last step, after every `await`. The error path (`responseHandler(e, …)` with a non-`Response`) only calls `reject`. Unchanged, on purpose: - **Non-2xx responses:** if reading the error body fails, the existing catch sets the body to `''` and the request still rejects with the HTTP status code. The caller already gets a rejection with the most useful information, so I left that branch alone. - **Streaming requests (`opts.stream`):** these go through `streamResponseHandler` and pipe the body into the returned stream. That path doesn't have this problem. ## Testing recommendations `npm test` passes: 238 tests, including 4 new ones in `test/nano.responsebody.test.js`: - JSON body breaks off → rejects (previously resolved `''`) - `text/plain` body breaks off → rejects (previously never settled; unhandled rejection) - `application/octet-stream` body breaks off → rejects (same as above) - complete `application/json` body that isn't valid JSON → still resolves `''` Without the change to `lib/nano.js`, the first three fail and the fourth passes. The new tests use a real `http.createServer` on `127.0.0.1` (random port) instead of undici's `MockAgent`. `MockAgent` always delivers the full reply body and then completes, so it can't simulate a connection that drops partway through the body. The server promises `content-length: 100`, writes part of the body, then destroys the socket. undici then raises the same `TypeError: terminated` (cause `SocketError: other side closed`) as it does against a real CouchDB. The file doesn't load `test/mock.js`, and `node --test` runs each file in its own process, so the other tests' global `MockAgent` (which has `disableNetConnect()`) doesn't apply here. Also checked: - The new tests pass on Node 20.20.2 (the `engines` minimum) and Node 24.21.0. - `standard` reports no errors in the new test file and no new errors in `lib/nano.js`. - `node --test --experimental-test-coverage` covers all changed lines. ## GitHub issue number Fixes #375 ## Related Pull Requests None. ## Checklist - [x] Code is written and works correctly; - [x] Changes are covered by tests; - [x] Documentation reflects the changes; (no documentation changes needed: this restores the documented promise semantics) This PR was prepared with the help of Claude Code (an AI coding assistant) and reviewed by me before submission. The commit carries a `Co-Authored-By` trailer, as recommended by the ASF generative tooling guidance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
