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

deno - replace deprecated Deno.run with Deno.Command #8893

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cscheid
Copy link
Collaborator

@cscheid cscheid commented Feb 26, 2024

Closes #8891.

@cscheid
Copy link
Collaborator Author

cscheid commented Feb 26, 2024

This is a bit obnoxious. Using Deno.Command crashes command.output() with a failed assertion deep into the Deno runtime:

Stack trace:
    at assert (ext:deno_web/00_infra.js:374:11)
    at readableStreamError (ext:deno_web/06_streams.js:2479:3)
    at readableStreamCollectIntoUint8Array (ext:deno_web/06_streams.js:1039:7)
    at eventLoopTick (ext:core/01_core.js:183:11)
    at async Promise.all (index 1)
    at async ChildProcess.output (ext:runtime/40_process.js:295:49)
    at async execProcess (file:///Users/cscheid/repos/github/quarto-dev/quarto-cli/src/core/process.ts:175:20)
    at async pandocListFormats (file:///Users/cscheid/repos/github/quarto-dev/quarto-cli/src/core/pandoc/pandoc-formats.ts:17:18)

(The CI output doesn't show this stack trace, I obtained it locally)

@cscheid
Copy link
Collaborator Author

cscheid commented Feb 26, 2024

We're blocked here until we upgrade Deno, I think.

But we're blocked on upgrading Deno until we confirm that the fix for denoland/deno#22180 has landed and resolves our issues. It seems that Deno v1.40.3 fixed that problem, so we should try again.

@cscheid cscheid mentioned this pull request Feb 27, 2024
@cscheid
Copy link
Collaborator Author

cscheid commented Feb 28, 2024

This PR is currently blocked by a Deno v1.41.0 bug with the following repro:

deno_repro.ts

async function processOutput(iterator: AsyncIterable<Uint8Array>): Promise<string> {
  const decoder = new TextDecoder();
  let outputText = "";
  for await (const chunk of iterator) {
    const text = decoder.decode(chunk);
    outputText += text;
  }
  return outputText;
}

async function* iteratorFromStream(stream: ReadableStream<Uint8Array>) {
  const reader = stream.getReader();
  try {
    while (true) {
      const { done, value } = await reader.read();
      if (done) return;
      yield value;
    }
  } finally {
    reader.releaseLock();
  }
}

const command = new Deno.Command("ls", {stdout: "piped", stderr: "piped"});
const process = command.spawn();

const promises: Promise<unknown>[] = [
  processOutput(iteratorFromStream(process.stdout)),
  processOutput(iteratorFromStream(process.stderr)),
];

await Promise.all(promises);

// assertion fails here
const status = await process.output();
$ deno run --allow-all deno_repro.ts
error: Uncaught (in promise) AssertionError: Assertion failed.
const status = await process.output();
               ^
    at assert (ext:deno_web/00_infra.js:374:11)
    at readableStreamError (ext:deno_web/06_streams.js:2529:3)
    at readableStreamCollectIntoUint8Array (ext:deno_web/06_streams.js:1081:7)
    at eventLoopTick (ext:core/01_core.js:153:7)
    at async Promise.all (index 1)
    at async ChildProcess.output (ext:runtime/40_process.js:322:49)
    at async file:///Users/cscheid/Desktop/daily-log/2024/02/28/deno_repro.ts:35:16

@cscheid
Copy link
Collaborator Author

cscheid commented Feb 28, 2024

Upstream deno bug report: denoland/deno#22621

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.

Replace Deno.run with Deno.command
1 participant