Skip to content

TS compat fixes based on reported lib compatibility issues - #120

Open
noise64 wants to merge 21 commits into
mainfrom
ts-lib-compat-fixes
Open

TS compat fixes based on reported lib compatibility issues#120
noise64 wants to merge 21 commits into
mainfrom
ts-lib-compat-fixes

Conversation

@noise64

@noise64 noise64 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
  • resolves GOL-60
  • resolves GOL-223
  • resolves GOL-324
  • resolves GOL-325
  • resolves GOL-346
  • fixes Request.clone()/bytes() and tees stream bodies in Response.clone()
  • lets TextDecoder/TextDecoderStream { fatal: true } reach the native strict decoder
  • accepts typed arrays in the native websocket and dgram send bindings, and sends blob chunks via Blob.arrayBuffer()
  • normalizes any ArrayBuffer view in fetch bodies, honoring byteOffset/byteLength
  • cancels the underlying WASI HTTP request on fetch abort with futures::future::Abortable, since rquickjs spawns that future detached from the returned promise
  • adds a functional golem:websocket test mock, a slow-response route, runtime tests, and regenerated dts goldenfiles

@blacksmith-sh

This comment has been minimized.

@blacksmith-sh

This comment has been minimized.

@noise64
noise64 marked this pull request as ready for review July 23, 2026 14:55
const nativeResponse = await request.receiveResponse();
let nativeResponse;
try {
nativeResponse = await request.receiveResponse(signal);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signal only guards receiveResponse(signal). Once a server returns a non-redirect response, its native listener is removed and the code waits on an unguarded bodyPromise. If the server responds before consuming a slow or infinite streaming request body, a later abort neither rejects the fetch nor cancels the upload.

The agent reproduced this with an endpoint returning 200 immediately and a request stream whose pull() never resolves. The abort fired, but the fetch did not receive its abort reason; the component eventually failed after about 21 seconds with a stalled-root-task panic.

Cancellation needs to remain active until the entire streaming request finishes and must actively cancel the stream reader/body writer—not merely restore the old outer Promise.race, which would leave native work detached.

reader.onload = () => {
// Blob support: read the blob's bytes and send them as binary.
// Uses Blob.arrayBuffer() directly (the runtime has no FileReader).
data.arrayBuffer().then((result) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blob transmission is deferred through arrayBuffer().then(...), while subsequent non-Blob sends execute synchronously. Thus:

ws.send(new Blob([bytes]));
ws.send('hello');

sends "hello" before the Blob. The agent tightened the PR’s new test to assert ordering; the recorded frames were:

Binary([1,2,3]), Binary([4,5,6]), Text("hello"), Binary([7,8,9])

although the Blob call preceded the text call. The current test uses contains, so it misses the regression. Outbound operations need a serialized send queue when asynchronous Blob conversion is involved.

...input._options,
};
this._options = { ...input._options };
// Copy the raw body directly (the body getters read `this._body`). Tee a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Request-copy path resets _bodyUsed and copies/tees _body without checking whether the source was already consumed:

const request = new Request(url, { method: 'POST', body: 'abc' });
await request.text();
request.clone(); // succeeds on PR head; must throw TypeError

A focused runtime repro failed because cloning succeeded. Node and the Fetch contract reject both request.clone() and new Request(request) after consumption. Check the source’s used/disturbed state before copying or teeing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants