MLE-32051 Fix multipart body termination for Node.js v26 compatibility

[jonmille]

Summary

Fixes client.documents.write() failing with ECONNRESET / socket hang-up when writing an array of documents on Node.js v26.x.

Closes GH #1104 | Jira: MLE-32051

---

Problem

When calling client.documents.write([doc1, doc2, ...]) on Node.js v26, the HTTP request body is never properly terminated. MarkLogic receives the part content but not the closing multipart boundary (--boundary--), and responds with:

HTTP 500: XDMP-MULTIPART-BOUNDARY: xdmp:get-request-part-body("json")
         -- Ending of the boundary is incorrect

The client then receives an ECONNRESET after ~30 seconds.

Root cause: multipart-stream 2.0.1 uses sandwich-stream (a Readable) to assemble the multipart body. For each part it creates a PassThrough stream, writes headers + CRNL + body content as separate .write() calls, and adds it to sandwich-stream. sandwich-stream's _currentStreamOnReadable calls read() to pull data from each sub-stream. On Node.js v26, read() returns only the first write-chunk from a PassThrough; the 'readable' event does not re-fire for subsequent chunks, leaving them stranded. Because _currentStreamOnEnd never triggers, sandwich-stream's _pushTail() / push(null) is never reached and the closing boundary is never emitted.

The bug affects Node.js v26.6.0 (the reporter's version) and v26.7.0. Node.js v24.x is not affected.

---

Fix

requester.js — bypasses sandwich-stream in both affected code paths:

• multipartRequester() — requestPartList path (client.documents.write(array)): detects whether any part carries stream content (typeof content.pipe === 'function').
  • All-synchronous content (the v26 bug scenario — array of document objects): the multipart body is assembled directly as incremental Buffer writes to the request, avoiding sandwich-stream entirely.
  • Stream content present (binary writes, large documents, Data Services ValueStream): falls back to multipart-stream with pipe(). Stream-driven PassThroughs receive writes asynchronously after listeners are attached, so the Node.js v26 'readable' regression does not affect them.
• chunkedMultipartRequester(): non-stream (metadata) parts are written to the request as Buffer chunks; the final streaming content part is forwarded via requestWriter.pipe(request, { end: false }) followed by the closing boundary and request.end().

The bindingParam and requestPartsProvider paths in multipartRequester() retain multipart-stream with pipe() and an explicit err => request.destroy(err) error handler to properly tear down the socket on stream errors.

---

Testing

Tested manually with repro.mjs (see MLE-32051):

Node.js	Before fix	After fix
v24.19.0	PASS	PASS
v26.6.0	FAIL (ECONNRESET after 30s)	PASS (0.39s)
v26.7.0	FAIL (ECONNRESET after 30s)	PASS (0.70s)

MarkLogic server: marklogic-server-ubi:11.3.6-ubi-2.2.6

Existing tests continue to pass with: `npx mocha test-basic/documents-core.js --timeout 0

[Copilot]

Pull request overview

This PR fixes client.documents.write([...]) failures on Node.js v26 by avoiding the sandwich-stream-based multipart body assembly that can strand chunks and prevent the final closing boundary from being emitted.

Changes:

• Reworks multipartRequester() to directly assemble the multipart body for the requestPartList path (array write) and end the request with a closing boundary.
• Reworks chunkedMultipartRequester() to write non-stream parts directly and then stream the final content part, followed by the closing boundary.

Suppressed comments (2)

lib/requester.js:487

• This manual 'data'/'end' forwarding bypasses stream backpressure and can lead to large in-memory buffering when the provider emits large bodies. Prefer piping the multipart stream into the request (and handle stream errors).

    multipartStream.on('data', chunk => request.write(chunk));
    multipartStream.on('end', () => request.end());

lib/requester.js:579

• Forwarding requestWriter 'data' events directly into request.write() bypasses stream backpressure and can buffer large streamed content in memory. Prefer piping requestWriter into the request with { end: false } and then append the closing boundary in the 'end' handler.

          requestWriter.on('data', chunk => request.write(chunk));
          requestWriter.on('end', () => {
            request.write(Buffer.from(CRNL + '--' + boundary + '--'));
            request.end();
          });

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[RitaChen609]

Can you add repro tests to the test suite in the repo?

[RitaChen609]

We don't need specific repro tests for this because the existing tests (test-basic/bindingFromParam.js, test-basic/optic-cts-param-test.js, etc) reproduce the same bug.

[RitaChen609]

@jonmille When I ran "npx mocha test-basic/bindingFromParam.js" on nodejs 26, 22 tests were failing. Apparently, the PR's fix for nodejs 26 is incomplete.

FYI Copilot analysis:

test-basic/bindingFromParam.js reproduces the same underlying multipart-boundary-termination bug from MLE-32051:

3 passing / 22 failing — all failures are 20000ms timeouts (hung write, never completed)

Root cause match: db.rows.query(plan, options, bindingParam) sets operation.bindingParam in rows.js:256, which routes through the operation.bindingParam branch of multipartRequester() in requester.js:427. Looking at the current PR code, this branch was left unchanged — it still relies entirely on multipart-stream's addPart() (wrapping the form-data object as a single part), which the PR's own root-cause analysis identifies as the buggy code path on Node v26.

Why even the "no attachment" tests hang: addPart() always writes the part's headers (Content-Type, Accept) as separate synchronous .write() calls to a fresh PassThrough, then pipes the form stream into it — the header writes alone are enough to trigger the same "only first buffered chunk survives, no further readable" regression, so every bindingParam-based query hangs, not just ones with binary/stream attachments.

Why exactly 2 tests pass: 'test without column' and 'test with wrong column type;' never reach the network layer at all — they throw synchronously during op.fromParam(...) argument validation (caught in a try/catch), so no multipart request is ever sent.

This is a more severe gap than the "mixed array" issue — it means the PR's fix is incomplete not just for an edge case, but for an entire, commonly-used feature (Optic fromParam bindings) on Node v26. Every real bindingParam write is broken.

[jonmille]

@jonmille When I ran "npx mocha test-basic/bindingFromParam.js" on nodejs 26, 22 tests were failing. Apparently, the PR's fix for nodejs 26 is incomplete.

FYI Copilot analysis:

test-basic/bindingFromParam.js reproduces the same underlying multipart-boundary-termination bug from MLE-32051:

3 passing / 22 failing — all failures are 20000ms timeouts (hung write, never completed)

Root cause match: db.rows.query(plan, options, bindingParam) sets operation.bindingParam in rows.js:256, which routes through the operation.bindingParam branch of multipartRequester() in requester.js:427. Looking at the current PR code, this branch was left unchanged — it still relies entirely on multipart-stream's addPart() (wrapping the form-data object as a single part), which the PR's own root-cause analysis identifies as the buggy code path on Node v26.

Why even the "no attachment" tests hang: addPart() always writes the part's headers (Content-Type, Accept) as separate synchronous .write() calls to a fresh PassThrough, then pipes the form stream into it — the header writes alone are enough to trigger the same "only first buffered chunk survives, no further readable" regression, so every bindingParam-based query hangs, not just ones with binary/stream attachments.

Why exactly 2 tests pass: 'test without column' and 'test with wrong column type;' never reach the network layer at all — they throw synchronously during op.fromParam(...) argument validation (caught in a try/catch), so no multipart request is ever sent.

This is a more severe gap than the "mixed array" issue — it means the PR's fix is incomplete not just for an edge case, but for an entire, commonly-used feature (Optic fromParam bindings) on Node v26. Every real bindingParam write is broken.

I also identified that these tests were also related to the changes, and I confirmed they are passing now:
[documents-core.js] (array + stream writes)
[bindingFromParam.js]
[docColTypes-test.js]
[lockForUpdate-test.js]
[optic-remove.js]
[transformDoc-test.js]
[write-test.js]

[RitaChen609]

@jonmille Thanks for the bindingParam fix — confirmed bindingFromParam.js now passes on Node v26.7.0. While re-testing, found two more issues worth addressing, but I think these two edge cases (mixed-content arrays, stream-typed binding attachments) should not block the PR since the existing test cases all passed rather than regressions of the reported bug. We can create a new JIRA ticket to address them later.

Copilot drafted the following summary.

1. requestPartList mixed-content arrays still hang on Node v26 (confirmed via test)
client.documents.write([...]) with an array that mixes plain content (e.g. a JSON doc) and streamed content (e.g. fs.createReadStream() for a binary doc) still hangs/times out on Node v26.7.0.

Root cause: in multipartRequester(), hasStreamContent is true whenever any part in the array streams, which routes the entire array — including the plain parts — back through multipart-stream's addPart():

if (hasStreamContent) {
  const multipartStream = new Multipart(boundary);
  ...
  multipartStream.addPart({ headers: headers, body: content });
  ...
  multipartStream.pipe(request);
}

addPart() always writes each part's headers + CRNL synchronously to a fresh PassThrough before piping/ending the body, regardless of whether that specific part is a stream — this is the same write pattern that triggers the Node v26 'readable' regression described in the PR. So a batch containing even one streamed part drags every other (plain) part back into the broken path.

Suggested fix: apply the same technique already used for bindingParam and chunkedMultipartRequester() here — write non-stream parts as Buffers directly to request, and only pipe the part(s) whose content is an actual stream, instead of falling back to multipart-stream for the whole array whenever any single part streams.

Recommended test case: an array write with one plain-content document and one stream-content document in the same call, e.g.: run on Node v26.x. and add in test-basic/documents-core.js under an array with mixed plain and stream content

db.documents.write([
  { uri: '...json', contentType: 'application/json', content: '{"key1":"value 1"}' },
  { uri: '...png',  contentType: 'image/png',        content: fs.createReadStream(binaryPath) }
])

2. bindingParam.getBuffer() fix may throw for stream-typed attachments
form.getBuffer() (in form-data) only handles Buffer/string entries:

if (Buffer.isBuffer(this._streams[i])) {
  dataBuffer = Buffer.concat([dataBuffer, this._streams[i]]);
} else {
  dataBuffer = Buffer.concat([dataBuffer, Buffer.from(this._streams[i])]); // throws for a stream
}

Nothing in rows.js/requester.js currently restricts bindingParam.attachments values to strings — if a caller passes an actual fs.createReadStream() (as opposed to a string) as an attachment value, form.append() wraps it as a DelayedStream, and getBuffer() will throw TypeError: The first argument must be of type string or an instance of Buffer... instead of streaming it, which previously worked via multipartStream.add({body: form}).pipe(request).

Suggested fix: either validate/reject non-string attachment values early with a clear error, or detect stream-typed attachments and fall back to the streaming path for that case.

Recommended test case: db.rows.query(plan, options, bindingParam) where an attachment value is fs.createReadStream(...) rather than a string, on Node v26.x, to confirm whether this is an actual supported/broken scenario.