denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.47k stars 5.33k forks source link

Accessing `conn.readable` affect the behavior of sending data through the connection #22258

Open lambdalisue opened 8 months ago

lambdalisue commented 8 months ago

Version: Deno 1.40.3

Somehow, using streams.toText (ReadableStream) instead of streams.readAll (Deno.Reader) affects the behavior of WritableStream of a Deno.Conn

Let's me explain this step by step.

The server read data from the client first. And then send data to the client.

If I use the following code to read the data from the client, sending data to the client also success.

const value = (new TextDecoder()).decode(await streams.readAll(conn));

But with the following code, sending data to the client fails with "Bad resource ID".

const value = await streams.toText(conn.readable);

The code for sending data to the client is

await ReadableStream
  .from("Hello from server")
  .pipeThrough(new TextEncoderStream())
  .pipeTo(conn.writable, {
    preventClose: true, // https://github.com/denoland/deno/issues/15442
  });

So not sure but just accessing conn.readable has some side-effects? I guess it is similar issue with https://github.com/denoland/deno/issues/15442 but I felt the situation is a bit different so I made a new one. Please close if it is duplicated issue.

What I've tried to avoid the error

Additional information

With streams.readAll (Success)
Receiving data from client...
Warning
├ Use of deprecated "readAll()" API.
│
├ This API will be removed in version 0.214.0 of the Deno Standard Library.
│
├ Suggestion: Import from `https://deno.land/std/io/read_all.ts` instead.
│
├ Suggestion: It appears this API is used by a remote dependency.
│             Try upgrading to the latest version of that dependency.
│
└ Stack trace:
  ├─ at Module.readAll (https://deno.land/std@0.213.0/streams/read_all.ts:39:12)
  ├─ at handleNewConnection (file:///Users/alisue/ghq/github.com/lambdalisue/deno-connect-issue/server.ts:8:58)
  ├─ at main (file:///Users/alisue/ghq/github.com/lambdalisue/deno-connect-issue/server.ts:53:5)
  └─ at eventLoopTick (ext:core/01_core.js:64:7)

Recieved Hello from client
Sending data to client...
Sent
Closing connection...
Closed
With streams.toText (Fail)
Receiving data from client...
Recieved Hello from client
Sending data to client...
error: Uncaught (in promise) BadResource: Bad resource ID
    at Object.write (ext:deno_web/06_streams.js:1155:20)
    at Module.invokeCallbackFunction (ext:deno_webidl/00_webidl.js:944:16)
    at WritableStreamDefaultController.writeAlgorithm (ext:deno_web/06_streams.js:3915:14)
    at writableStreamDefaultControllerProcessWrite (ext:deno_web/06_streams.js:4485:55)
    at writableStreamDefaultControllerAdvanceQueueIfNeeded (ext:deno_web/06_streams.js:4388:5)
    at writableStreamDefaultControllerWrite (ext:deno_web/06_streams.js:4532:3)
    at writableStreamDefaultWriterWrite (ext:deno_web/06_streams.js:4679:3)
    at Object.chunkSteps (ext:deno_web/06_streams.js:2743:17)
    at readableStreamFulfillReadRequest (ext:deno_web/06_streams.js:2585:17)
    at readableStreamDefaultControllerEnqueue (ext:deno_web/06_streams.js:1764:5)
server.ts
import * as streams from "https://deno.land/std@0.213.0/streams/mod.ts";

async function handleNewConnection(conn: Deno.Conn) {
  console.log("Receiving data from client...");
  ////////////////////////////////////////////////////////////////////////////////////////

  // Read data with `readAll` seems OK
  //const value = (new TextDecoder()).decode(await streams.readAll(conn));

  // But read data with `streams.toText` cause the error when try to send data to client
  const value = await streams.toText(conn.readable);

  ////////////////////////////////////////////////////////////////////////////////////////
  console.log("Recieved", value);

  console.log("Sending data to client...");

  ////////////////////////////////////////////////////////////////////////////////////////
  //
  // Sending data to client will fail with the following error when `streams.toText` is
  // used to read data from client previously.
  //
  // error: Uncaught (in promise) BadResource: Bad resource ID
  //   at Object.write (ext:deno_web/06_streams.js:1155:20)
  //   at Module.invokeCallbackFunction (ext:deno_webidl/00_webidl.js:944:16)
  //   at WritableStreamDefaultController.writeAlgorithm (ext:deno_web/06_streams.js:3915:14)
  //   at writableStreamDefaultControllerProcessWrite (ext:deno_web/06_streams.js:4485:55)
  //   at writableStreamDefaultControllerAdvanceQueueIfNeeded (ext:deno_web/06_streams.js:4388:5)
  //   at writableStreamDefaultControllerWrite (ext:deno_web/06_streams.js:4532:3)
  //   at writableStreamDefaultWriterWrite (ext:deno_web/06_streams.js:4679:3)
  //   at Object.chunkSteps (ext:deno_web/06_streams.js:2743:17)
  //   at readableStreamFulfillReadRequest (ext:deno_web/06_streams.js:2585:17)
  //   at readableStreamDefaultControllerEnqueue (ext:deno_web/06_streams.js:1764:5)
  //
  ////////////////////////////////////////////////////////////////////////////////////////
  await ReadableStream
    .from("Hello from server")
    .pipeThrough(new TextEncoderStream())
    .pipeTo(conn.writable, {
      preventClose: true,
    });

  console.log("Sent");

  console.log("Closing connection...");
  conn.close();
  console.log("Closed");
}

async function main() {
  const listener = Deno.listen({ hostname: "localhost", port: 23232 });
  for await (const conn of listener) {
    handleNewConnection(conn);
  }
}

if (import.meta.main) {
  main();
}
client.ts
import * as streams from "https://deno.land/std@0.214.0/streams/mod.ts";

async function main() {
  console.log("Connecting to server...");
  const conn = await Deno.connect({ hostname: "localhost", port: 23232 });
  console.log("Connected");

  console.log("Sending data to server...");
  await ReadableStream
    .from("Hello from client")
    .pipeThrough(new TextEncoderStream())
    .pipeTo(conn.writable, {
      // It seems if the writable is closed, the connection is also closed.
      // Not exactly same but similar behavior is reported for FsFile.
      // https://github.com/denoland/deno/issues/15442
      preventClose: true,
    });
  // Use closeWrite() to send EOF to the server
  await conn.closeWrite();
  console.log("Sent");

  console.log("Receiving data from server...");
  // Read data from the server until EOF is received.
  const value = await streams.toText(conn.readable);
  console.log("Received", value);
}

if (import.meta.main) {
  main();
}
lucacasonato commented 4 months ago

Thank you for opening this. This is what is happening:

  1. The server receives a connection from the client.
  2. The client writes some data to the connection.
  3. The client half closes the connection using closeWrite().
  4. The server is reading the data from the connection using conn.readable. The behaviour of this readable is that when the underlying resource returns EOF, the resource gets closed entirely. This is good behaviour for most cases, especially in the contexts of FsFile - more on this below.
  5. The server now tries to send data. This fails because the resource was closed by the readable stream when EOF was encountered.

The reason that we should continue to automatically close the resource on readable streams when EOF is reached, is because it prevents you from leaking resources in the majority of usecases. An example:

Deno.serve(async () => {
  const file = await Deno.open("./README.md");
  return new Response(file.readable);
});

The underlying resource being closed here automatically at EOF is good behaviour - it prevents users from leaking the file resource.

However there are use cases where you may still want to write after encountering EOF - like the use case you describe here. I think for this use case, we should add an option to Deno.connect / Deno.open that allows you to specify whether the resource should be closed on encountering EOF.

namespace Deno {
function open(path: string | URL, options: { preventCloseOnEOF: boolean }): Deno.FsFile;
function connect(addr: Deno.Addr, options: { preventCloseOnEOF: boolean }): Deno.TcpConn;
function connectTls(addr: Deno.Addr, options: { preventCloseOnEOF: boolean }): Deno.TlsConn;
function startTls(conn: Deno.TcpConn, options: { preventCloseOnEOF: boolean }): Deno.TlsConn;
};