[🐞] Error on response attempting to close WritableStream with Bun adapter

[octet-stream]

Which component is affected?

Qwik City (routing)

Describe the bug

When using Qwik with Bun adapter (Bun.serve) with disabled SSG, the server throws an error attempting to close locked WritableStream. From what I can tell, the stream becomes locked after the pipeTo call here and it stays on that stage when writebleStream.close() is called here. The server itself however responds without any issue, this error for some reason just shows up in logs, but I get this during every server response response.

Reproduction

https://github.com/octet-stream/qwik-bun-stream-issue

Steps to reproduce

1. Clone this repository;
2. Install dependencies via bun i;
3. Build the project for production bun run build;
4. Start the app via bun run serve;
5. Open http://localhost:3000 in your browser;

After the main page shows up, look at the terminal and you'll see following error:

$ bun server/entry.bun.js
Server started: http://localhost:3000/
636 |     } finally {
637 |       await stream.ready;
638 |       await stream.close();
639 |       await pipe;
640 |     }
641 |     await writableStream.close();
                          ^
TypeError: WritableStream.close method can only be used on non locked WritableStream
      at writableStreamCloseForBindings (:1:21)
      at /Users/octetstream/projects/qwik-bun-stream-issue/server/entry.bun.js:641:11

System Info

System:
    OS: macOS 14.3.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 62.06 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.6.2 - ~/Library/Caches/fnm_multishells/2006_1708258753953/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.2.4 - ~/Library/Caches/fnm_multishells/2006_1708258753953/bin/npm
    pnpm: 8.15.4 - /opt/homebrew/bin/pnpm
    bun: 1.0.29 - /opt/homebrew/bin/bun
  Browsers:
    Chrome: 122.0.6261.94
    Safari: 17.3.1
  npmPackages:
    @builder.io/qwik: ^1.4.5 => 1.4.5 
    @builder.io/qwik-city: ^1.4.5 => 1.4.5 
    undici: * => 6.6.2 
    vite: ^5.0.12 => 5.1.4

Additional Information

No response

[gavzla]

+1 same behavior here

[devcaeg]

@wmertens Do you know if this error is caused by Bun or by Qwik?

[devcaeg]

The error occurs in this line of code

For some reason, at that line, "writableStream" is in "locked" state, and therefore cannot be closed.

[octet-stream]

Do you know if this error is caused by Bun or by Qwik?

I'm not sure, but I tried Qwik with Node.js servers and I don't get this error, so it might be.

[wmertens]

Well, open for PRs :)

[devcaeg]

If before executing await writableStream.close(); it is verified that it is not in locked state, the error would be avoided. But I am not sure if this could cause other errors.

if (!writableStream.locked) {
    await writableStream.close();
}

[PatrickJS]

well if lock is true then it will throw an error correct? unless this error will only throw in bun

[devcaeg]

Yes, if locked is true, then an error will occur when trying to execute close(). The same happens if an attempt is made to execute getWriter() when locked is true.

[PatrickJS]

what about

if (writableStream.locked) {
    writableStream.releaseLock();
}
await writableStream.close();

[PatrickJS]

ChatGPT 😏 suggested this after I asked it to account for every gotcha haha

export function renderQwikMiddleware(render: Render) {
  return async (requestEv: RequestEvent) => {
    // Your existing code...

    try {
      // Your rendering logic...

    } finally {
      try {
        await stream.ready;
        await stream.close();
        console.log('Writer closed successfully');
      } catch (error) {
        console.error('Error closing the writer', error);
      }

      try {
        await pipe;
        console.log('pipe operation completed successfully');
      } catch (error) {
        console.error('Error in pipe operation', error);
        if (writableStream.abort) {
          writableStream.abort(error);
        }
      }

      if (!writableStream.locked) {
        try {
          await writableStream.close();
          console.log('Stream closed successfully');
        } catch (error) {
          console.error('Error occurred when closing the writable stream', error);
        }
      } else {
        console.error('The writable stream is locked.');
        // Wait 2 seconds
        const waitTime = 2000;
        await new Promise(resolve => setTimeout(resolve, waitTime));

        if (writableStream.locked) {
          try {
            stream.releaseLock();
            console.log('The writer has been released, trying to close the stream again');
            await writableStream.close();
          } catch (error) {
            console.error('Error occurred when forcibly trying to close the writable stream', error);
          }
        }
      }
    }
  };
}

[devcaeg]

In this WritableStream does not have the relaseLock() function.

WritableStream {
  locked: true,
  abort: [Function: abort],
  close: [Function: close],
  getWriter: [Function: getWriter],
}

[PatrickJS]

maybe type is outdated?
https://nodejs.org/api/webstreams.html#readablestreamdefaultreaderreleaselock
https://developer.mozilla.org/en-US/docs/Web/API/WritableStreamDefaultWriter/releaseLock

[PatrickJS]

oh i see ChatGPT got releaseLock from stream when you getWriter()

            writableStream.getWriter().releaseLock();
            console.log('The writer has been released, trying to close the stream again');
            await writableStream.close();

[PatrickJS]

even if we do this it's weird that it's only bun. maybe something else is wrong. we have another team using bun with no issues

[devcaeg]

It does not seem to be a type problem, since I try to execute releaseLock (both in writableStream and stream) and I get the error that this function does not exist.

[devcaeg]

Maybe I'm wrong, but I think they are different.

https://developer.mozilla.org/en-US/docs/Web/API/WritableStreamDefaultWriter

https://developer.mozilla.org/en-US/docs/Web/API/WritableStream

[devcaeg]

oh i see ChatGPT got releaseLock from stream when you getWriter()

            writableStream.getWriter().releaseLock();
            console.log('The writer has been released, trying to close the stream again');
            await writableStream.close();

The problem is when trying to execute getWriter() when locked is true, this causes an error.

[octet-stream]

even if we do this it's weird that it's only bun. maybe something else is wrong. we have another team using bun with no issues

Do they use Bun.serve as production server with Qwik? With which version of Bun and Qwik?

[PatrickJS]

@devcaeg you can access the reference with stream.releaseLock();

[devcaeg]

@devcaeg you can access the reference with stream.releaseLock();

Are you using Node or Bun? Because maybe Bun for some reason does not incorporate releaseLock().

[PatrickJS]

I'm going to download the repro and try it

[PatrickJS]

yeah it looks like bun doesn't have releaseLock

[octet-stream]

I see this method on WritableStream on v1.1.8

[PatrickJS]

I tried 1.1.8 not working. maybe we need to ping bun guys

[devcaeg]

I wrote in Bun discord and Jarred replied the following:

releaseLock is a property of ReadableStreamDefaultReader, not on WritableStream afaik

[PatrickJS]

makes sense

[devcaeg]

https://bun.sh/docs/runtime/nodejs-apis#textencoderstream 😅

[octet-stream]

Let's figure out if the issue occurs because of this polyfill first :)

[octet-stream]

Here's the polyfill:

qwik/packages/qwik-city/middleware/bun/index.ts

Lines 21 to 53 in bb8b7a9

	class TextEncoderStream {
	// minimal polyfill implementation of TextEncoderStream
	// since bun does not yet support TextEncoderStream
	_writer: any;
	readable: any;
	writable: any;
	constructor() {
	this._writer = null;
	this.readable = {
	pipeTo: (writableStream: any) => {
	this._writer = writableStream.getWriter();
	},
	};
	this.writable = {
	getWriter: () => {
	if (!this._writer) {
	throw new Error('No writable stream');
	}
	const encoder = new TextEncoder();
	return {
	write: async (chunk: any) => {
	if (chunk != null) {
	await this._writer.write(encoder.encode(chunk));
	}
	},
	close: () => this._writer.close(),
	ready: resolved,
	};
	},
	};
	}
	}

I see from the usage of it that it should return a value, which this polyfill is not doing.

[PatrickJS]

yeah I asked chatgpt to give me a better polyfil and it works

[PatrickJS]

the chatgpt version was

var TextEncoderStream = class {
  constructor() {
    this._encoder = new TextEncoder();
    this._writer = null;
    this.ready = Promise.resolve();
    this.reader = null;
    this.closed = false;

    this.readable = new ReadableStream({
      start: controller => {
        this.reader = controller;
      }
    });

    this.writable = new WritableStream({
      write: async (chunk) => {
        if (chunk != null && this.reader) {
          let encoded = this._encoder.encode(chunk);
          this.reader.enqueue(encoded);
        }
      },
      close: () => {
        this.reader.close();
        this.closed = true;
      },
      abort: (reason) => {
        this.reader.error(reason);
        this.closed = true;
      }
    });
  }
};

[octet-stream]

Yes, it seem to work.

[PatrickJS]

@octet-stream can you make a pr (you can edit in github directly) and I'll update the pr if needed

[octet-stream]

Sure, why not.

[devcaeg]

Great! I didn't realize that Qwik had this polyfill to make Bun work.

[PatrickJS]

yup i didn't realize either

[octet-stream]

I actually did, when I was investigating this problem back March, but I completely forgot to mention it in this issue, sorry.

[PatrickJS]

here is a pr example #6308

I want you to make a pr so you guys get contributes 😁

[octet-stream]

Here comes the PR :)

[devcaeg]

I am satisfied with the simple fact that the error is solved, haha. Thank you both very much.

[PatrickJS]

@devcaeg @octet-stream
we want to get a few more features in before the next release. so what you can do is add the polyfill yourself before createQwikCity runs and you should be good to go

[octet-stream]

You mean the polyfill will be exposed from Qwik to be installed manually? This is fine by me, but it must be documented then.

[PatrickJS]

no I meant for now until the next release. in your codebase look where createQwikCity is ran then above the file define TextEncoderStream globally on globalThis.TextEncoderStream if it's not defined

[octet-stream]

Ah, sure.

[PatrickJS]

yeah, that way you guys aren't blocked and waiting for the next release 👍

[devcaeg]

Thank you!

[octet-stream]

Can confirm this works in my project. Thank you for your help!

[octet-stream]

By the way, here's the issue in Bun asking for TextEncoderStrea/TextDecorerStream oven-sh/bun#5648 I'll comment it here to add a reference. There's even a polyfill: oven-sh/bun#5648 (comment)

[PatrickJS]

oh awesome they had a polyfill

[PatrickJS]

ok ChatGPT make the encoder version based on the code in that bun issue decoder version

add this at the top of your entry.bun.ts file

export class _TextEncoderStream_polyfill extends TransformStream<
  string,
  Uint8Array
> {
  encoding: string = "utf-8";
  constructor() {
    const encoder = new TextEncoder();
    super({
      transform(
        chunk: string,
        controller: TransformStreamDefaultController<Uint8Array>
      ) {
        const encoded = encoder.encode(chunk);
        if (encoded.byteLength > 0) {
          controller.enqueue(encoded);
        }
      },
      flush() {
        // When the stream is finished, we don't have any cleanup to do.
        // We don't need to enqueue any data because TextEncoder.encode doesn't have any state.
      },
    });
  }
}

globalThis.TextEncoderStream =
  globalThis.TextEncoderStream ||
  class TextEncoderStream extends _TextEncoderStream_polyfill {};

[devcaeg]

I think the latter is better. Anyway, the best thing is for Bun to add compatibility, hopefully it won't take too long.

[devcaeg]

In the latest versions of Bun, TextEncoderStream and TextDecoderStream compatibility has been added, so this polyfill is no longer necessary.

[gioboa]

Thanks for the update