Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@std/io: readAllSync only reads a single line from Deno.stdin when stdin is interactive #6346

Closed
frou opened this issue Jan 14, 2025 · 4 comments · Fixed by #6355
Closed
Labels
bug Something isn't working upstream Changes upstream are required to solve these issues

Comments

@frou
Copy link

frou commented Jan 14, 2025

https://jsr.io/@std/io/doc/~/readAllSync

Steps to Reproduce

#!/usr/bin/env deno run

import { readAllSync } from "@std/io"

const content = readAllSync(Deno.stdin)

console.log()
console.log(content.length)

Expected behavior

When running the above script in a shell session, I'd expect to be able to do the following (typing 3 lines then pressing Ctrl-D (i.e. EOF) to finish):

$ ./repro.ts
one
two
three
^D

14
$

That is how all the other languages I've tried work.

Describe the bug

But readAllSync returns after I type the first line.

$ ./repro.ts
one

4
$

Environment

  • OS: macOS 12.7.6
  • deno version: 2.1.5
  • std version: 0.225.0
@frou frou added bug Something isn't working needs triage labels Jan 14, 2025
@kt3k
Copy link
Member

kt3k commented Jan 21, 2025

hmm, it looks like 0 byte read is translated to null (EOF) in Deno CLI (ref: https://github.com/denoland/deno/blob/4e0bf4b093c12f6baceee960e5c07bbc13b69a1e/ext/io/12_io.js#L38 ) and that seems causing this issue. I don't think there's anything we can fix in STD.

@kt3k kt3k added upstream Changes upstream are required to solve these issues and removed needs triage labels Jan 21, 2025
@0f-0b
Copy link
Contributor

0f-0b commented Jan 21, 2025

Here's a failing test which demonstrates that this is a bug in readAllSync.

import { assertEquals } from "jsr:@std/assert@1/equals";
import {
  readAll,
  readAllSync,
  type Reader,
  type ReaderSync,
} from "jsr:@std/[email protected]";

class SlowBuffer implements Reader, ReaderSync {
  #remaining: Uint8Array;

  constructor(bytes: Uint8Array) {
    this.#remaining = bytes;
  }

  // deno-lint-ignore require-await -- implementing `Reader` interface
  async read(p: Uint8Array): Promise<number | null> {
    return this.readSync(p);
  }

  readSync(p: Uint8Array): number | null {
    if (p.length === 0) {
      throw new TypeError("p is empty");
    }
    const remaining = this.#remaining;
    if (remaining.length === 0) {
      // no more bytes to read; signal end-of-stream
      return null;
    }
    // read one byte at a time
    this.#remaining = remaining.subarray(1);
    p[0] = remaining[0];
    return 1;
  }
}

Deno.test("readAll() with SlowBuffer", async () => {
  const testBytes = crypto.getRandomValues(new Uint8Array(20));
  const reader = new SlowBuffer(testBytes);
  const actualBytes = await readAll(reader);
  assertEquals(actualBytes, testBytes);
});

Deno.test("readAllSync() with SlowBuffer", () => {
  const testBytes = crypto.getRandomValues(new Uint8Array(20));
  const reader = new SlowBuffer(testBytes);
  const actualBytes = readAllSync(reader);
  assertEquals(actualBytes, testBytes);
});

@kt3k
Copy link
Member

kt3k commented Jan 22, 2025

Oh, this actually looks like a bug of readSyncAll. Thanks for the test case.

@kt3k
Copy link
Member

kt3k commented Jan 22, 2025

Looks like this was introduced in #4128 when we re-implemented readAllSync without using Buffer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Changes upstream are required to solve these issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants