dsherret / dax

Cross-platform shell tools for Deno and Node.js inspired by zx.
MIT License
967 stars 33 forks source link

.abort() not working properly when running deno task <task-name> #146

Closed Hexagon closed 1 year ago

Hexagon commented 1 year ago

Ran into a problem on a production pup instance running a web server using command deno task prod. Once started, the server cannot be killed programatically. Pup uses dax internally.

The problem is solved by starting the server using command deno run instead of deno task.

When aborting deno task prod using CTRL+C at the command line, everything works as it should.

Complete repro at https://github.com/Hexagon/dax-issue-146

Repro steps

Running deno run -A main.ts directly using dax works fine

# First start - all good, 
> deno run -A works.ts
Listening...
... aborted with exit code 124
# Second start - all good
> deno run -A works.ts
Listening...
... aborted with exit code 124

Running deno task prod using dax seem to leave the actual deno process hanging

# First start - all good, running deno task prod using dax
> deno run -A failure.ts
Listening...
... aborted with exit code 124
# Second start - the server from the first start is still listening on the port, even though everything see fine?
> deno run -A failure.ts
AddrInUse: Address already in use
... aborted with exit code 124

Code for repro

deno.json

{
    "tasks": {
        "prod": "deno run -A main.ts"
    }
}

main.ts

const server = Deno.listen({ port: 8083 });
console.log(`HTTP webserver running.  Access it at:  http://localhost:8080/`);

/* ... the rest of the example server imlementation */

works.ts

import $ from "https://deno.land/x/dax/mod.ts";

const child = $`deno run -A main.ts`.spawn();

// abort the child after 3s
await $.sleep("1s");
child.abort();

await child; // Error: Aborted with exit code: 124

failure.ts

import $ from "https://deno.land/x/dax/mod.ts";

const child = $`deno task prod`.spawn();

// abort the child after 3s
await $.sleep("1s");
child.abort();

await child; // Error: Aborted with exit code: 124
sigmaSd commented 1 year ago

I can't reproduce on linux, I can run failure.ts many times without error

Hexagon commented 1 year ago

Are you sure? The reproduction code fail on both a physical linux machine and codespaces for me.

Codespaces:

deno 1.34.1 (release, x86_64-unknown-linux-gnu)
v8 11.5.150.2
typescript 5.0.4

Local:

deno 1.33.4 (release, x86_64-unknown-linux-gnu)
v8 11.4.183.2
typescript 5.0.4

I made a repo out of it to keep it simple

https://github.com/Hexagon/dax-issue-146

sigmaSd commented 1 year ago

yeah the repo you linked also work

but funny thing I found, turns out deno lsp listen to port 8084 by default, so that might be the issue ? can you try using a different port ?

sigmaSd commented 1 year ago

or maybe no, let me pr some fixes to that repo, so we can test there more

sigmaSd commented 1 year ago

Can you try here https://github.com/sigmaSd/dax-issue-146 (this makes sure that dax have the latest version) and can you kill -9 deno before starting just for sanity check

Hexagon commented 1 year ago

Aaah, your version work fine. But it is because the server exit gracefully before dax tries to kill the process, if I add the server code to your version, it starts failing again (because the process is kept alive until it's killed by dax).


// Connections to the server will be yielded up as an async iterable.
for await (const conn of server) {
    // In order to not be blocking, we need to handle each connection individually
    // without awaiting the function
    serveHttp(conn);
  }

  async function serveHttp(conn: Deno.Conn) {
    // This "upgrades" a network connection into an HTTP connection.
    const httpConn = Deno.serveHttp(conn);
    // Each request sent over the HTTP connection will be yielded as an async
    // iterator from the HTTP connection.
    for await (const requestEvent of httpConn) {
      // The native HTTP server uses the web standard `Request` and `Response`
      // objects.
      const body = `Your user-agent is:\n\n${
        requestEvent.request.headers.get("user-agent") ?? "Unknown"
      }`;
      // The requestEvent's `.respondWith()` method is how we send the response
      // back to the client.
      requestEvent.respondWith(
        new Response(body, {
          status: 200,
        }),
      );
    }
  }
sigmaSd commented 1 year ago

I see, this is an issue with abortcontroller , here is a minimal example, I think you should close this issue and open one against deno repo

const control = new AbortController();
const child = new Deno.Command("deno", {
  // args: ["run","-A", "main.ts"], // works
  args: ["task", "prod"], // fails
  signal: control.signal,
}).spawn();

await new Promise((r) => setTimeout(r, 1000));
control.abort();

await child.status;
sigmaSd commented 1 year ago

or maybe its an issue with how deno task works

sigmaSd commented 1 year ago

actually here is the issue already https://github.com/denoland/deno/issues/18445

Hexagon commented 1 year ago

Ooh, cool. Thanks for checking it up :100: