go-task / task

A task runner / simpler Make alternative written in Go
https://taskfile.dev
MIT License
11.43k stars 617 forks source link

bug: parallel with sources and ignore_error causes other tasks to exit #704

Open lrstanley opened 2 years ago

lrstanley commented 2 years ago

Running this command:

$ task --parallel frontend debug --watch --taskfile task-test.yaml

And the below Taskfile:

version: "3"

output: prefixed

tasks:
  frontend:
    run: when_changed
    cmds:
      - while true;do echo "test"; sleep 1;done
  debug:
    ignore_error: true
    cmds:
      - sleep 4
      - exit 1
    sources:
      - "**/*.go"

I'm expecting the frontend (which would be a watcher in-itself and nodejs dev server), and debug (which would likely be go run *.go, I'd like debug to be able to fail, and not cause frontend to exit. If the command itself fails (the exit 1) as shown above, everything works as expected:

$ task --parallel frontend debug --watch --taskfile task-test.yaml
task: Started watching for tasks: frontend, debug
task: [frontend] while true;do echo "test"; sleep 1;done
[frontend] test
task: [debug] sleep 4
[frontend] test
[frontend] test
[frontend] test
[frontend] test
task: [debug] exit 1
[frontend] test
[frontend] test
[frontend] test
[...]

However, with the sources block being in place, if I change a .go file, and then the exit 1 causes an error to occur, frontend does not keep running.

$ task --parallel frontend debug --watch --taskfile task-test.yaml
task: Started watching for tasks: frontend, debug
task: [frontend] while true;do echo "test"; sleep 1;done
[frontend] test
task: [debug] sleep 4
[frontend] test
[frontend] test
[frontend] test
[frontend] test
task: [debug] exit 1  <-- exits due to error, but keeps going
[frontend] test
[frontend] test
[frontend] test
[frontend] test
[frontend] test
task: [debug] sleep 4 <-- re-triggered due to source file changes (changed a .go file)
task: [debug] exit 1

[...] <-- it killed frontend, and hangs until .go files are changed again.

I've also confirmed that even if the command is successful, nothing re-kicks off frontend.

Lastly, this does not seem to occur when not using when_changed on frontend (e.g. always causes it to spin back up), though this is obviously not ideal. I also understand that in the above example, I'm using when_changed with no sources/generates. Even with that, though, I'd prefer a task with ignore_error to not affect the other running non-dependency-related tasks, when using --parallel, as it's likely that they're completely unrelated (e.g. frontend webserver and backend webserver). That makes the most sense to me.

Thoughts?

lrstanley commented 2 years ago

Also seems as though with --watch, it doesn't actually stop the previously running process when it re-initiates the task commands. Leading to leaking processes running, if they don't complete when --watch notices a file change.

EDIT: and it also doesn't run any defer steps between the task re-invocation, when sources has changed. Only seems to run defer if the prior command actually failed. Not sure if this is intended behavior?

kevpie commented 2 years ago

@lrstanley I wanted to this exact thing, so thank your for this. It seems like removing run: when_changed will work how you intended.

I added echo $$ and $PPID to hopefully show the task isn't restarting (assuming the env evaluation is happening where I think it is).

version: "3"

output: prefixed

tasks:
  frontend:
#    run: when_changed
    cmds:
      - while true;do echo "test $$ - $PPID"; sleep 1;done
  debug:
    ignore_error: true
    cmds:
      - sleep 4
      - exit 1
    sources:
      - "**/*.go"
$ task --parallel frontend debug --watch
task: Started watching for tasks: frontend, debug
task: [frontend] while true;do echo "test $$ - $PPID"; sleep 1;done
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
task: [debug] sleep 4
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
task: [debug] exit 1
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
task: [frontend] while true;do echo "test $$ - $PPID"; sleep 1;done
[frontend] test 52827 - 65926
task: [debug] sleep 4
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
task: [debug] exit 1
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
[frontend] test 52827 - 65926
kevpie commented 2 years ago

Maybe I'm wrong as the log entry happens again for the frontend.

kevpie commented 2 years ago

Sorry for the spam... It looks like the frontend task needs sources or some reason to know not to restart...

version: "3"

output: prefixed

tasks:
  frontend:
    cmds:
      - while true;do echo "test $$ - $PPID"; sleep 1;done
    sources:
      - src/**/*.js
      - src/**/*.ts
  debug:
    ignore_error: true
    cmds:
      - sleep 4
      - exit 1
    sources:
      - "**/*.go"
$ task --parallel frontend debug --watch
task: Started watching for tasks: frontend, debug
task: [frontend] while true;do echo "test $$ - $PPID"; sleep 1;done
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
task: [debug] sleep 4
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
task: [debug] exit 1
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
[frontend] test 54669 - 65926
task: Task "frontend" is up to date
task: [debug] sleep 4
task: [debug] exit 1
task: Task "frontend" is up to date
task: [debug] sleep 4
^Ctask: [debug] exit 1

Triggering with

echo foo > foo.go

Now we can see Task deciding to not touch "frontend" Keep in mind you'll want to watch something other than your ts/js sources as yarn start etc will be watching already.

lrstanley commented 2 years ago

I've settled on the following to solve the defer-not-running problem, with --watch -- checking if the existing process is running, and sending a SIGTERM to it, before running it again, and waiting for it to gracefully exit. Not ideal, but works for my usecase, it'd be running a main package via go run:

    cmds:
      - while kill $(pgrep http 2>/dev/null) 2>/dev/null;do sleep 1;done
      - go run *.go --debug

As for the others, even explicitly providing a source to listen to, for the frontend, still isn't ideal for me. idk.

kevpie commented 2 years ago

I'm trying to solve similar problems except I have the firebase emulator thrown in. Looks like using run: once was helping, but any progress I make results in strange behavior. My new thought... Have Task launch Task to separate the run times for the different watched tasks. Feels hackie. I think there is some room for an idea on how to use Task as a way to run all your dev dependencies in watch mode.

kevpie commented 2 years ago

As I became frustrated with doing multiple watches, I've settled on a pattern that seems to clear up all my issues. It's a bit funky, but the idea is I set all of the parallel tasks for serving as a dependency so they get launched in parallel automatically. Then I have each make sure the don't exit on their own, using sleep <forlongtime> There is a defer or precondition that kills all related procs to the expected LISTENING ports. Finally the tasks that I expect to Watch on with changes get wrapped in a thing-watch task that calls the task using a new execution of Task with --watch there. This delegates the watch logic into another running instance of Task for each watched task.

Taskfile.yml

tasks:
  default:
    cmds:
      - task: serve
  functions-watch:
    - task functions:build --watch
  backend-watch:
    - task backend:serve --watch
  serve:
    deps:
      - ngrok:serve             # public https tunnels
      - stripe:serve            # stripe webhook enabled/disable - curl + sleep
      - firebase:sync-firestore # sync some data to  firestore - curl with retries till succeeds
      - firebase:serve          # emulators - pubsub,functions,auth
      - frontend:serve          #  yarn start
      - functions-watch         # yarn build Typescript functions
      - backend-watch           # go run .
...

backend.yml:

tasks:
  serve;
    cmds:
      - go run .
    sources:
      - "**/*.go"
    deps:
      - install
...

functions.yml:

tasks:
  build:
    cmds:
      - yarn build
    sources:
      - "src/**/*.ts"
    deps:
      - install
...

stripe.yml - example using sleep.

tasks:
  serve:
    run: once
    preconditions:
      - sh: |
          if [[ -z "{{.STRIPE_WEBHOOK_TEST_DEV}}" ]]; then echo STRIPE_WEBHOOK_TEST_DEV not set in .dev.local; exit 1; fi
          if [[ -z "{{.STRIPE_KEY}}" ]]; then echo STRIPE_KEY not set in .dev.local; exit 1; fi
    cmds:
      - defer: >
          curl --no-progress-meter https://api.stripe.com/v1/webhook_endpoints/{{.STRIPE_WEBHOOK_TEST_DEV}}
          -u {{.STRIPE_KEY}}:
          -d disabled=true
      - >
        curl --no-progress-meter https://api.stripe.com/v1/webhook_endpoints/{{.STRIPE_WEBHOOK_TEST_DEV}}
        -u {{.STRIPE_KEY}}:
        -d disabled=false
      - sleep 36000

Stopping services that should have been stopped by port

The precondition can also be used as a defer.

version: "3"

vars:
  FIREBASE_VERSION: 10.2.2
  _FIREBASE_VERSION: { sh: firebase --version }
tasks:
  tools:
    preconditions:
      - sh: "[[ {{.FIREBASE_VERSION == {{._FIREBASE_VERSION}} ]]"
        msg: "expected: {{.FIREBASE_VERSION}}, got: {{._FIREBASE_VERSION}}"
  serve:
    run: once
    preconditions:
      - sh: |         
          for x in 4000 4400 4500 5000 5001 8080 8085 9099; do
            for y in `lsof -i :"$PORT" | awk '{print $2}' | grep -v PID`; do
              echo "Killing process " $y
              kill -9 ${y}
            done
          done
    cmds:
      - yarn start-emulator
...

In all these cases the idea is each is a long running dependency that can respond to signals, either via the defer or the signal directly.

ghostsquad commented 2 years ago

Looking at this holistically, the goal of all this is to have task be a watcher for multiple things (requiring them to be started in parallel).

Also, your have a requirement that, one such task is allowed to fail, but should keep watching.

It sounds like maybe having a --watch flag on the task cli is ambiguous, and this should instead be a flag of the task itself?

Though, it's also unclear, if watch should rerun it's deps or not each time it restarts.

Additionally, another question is raised, which is whether or not defer should fire each time the task definition (which restarts because of watch) ends, or should it fire only once after the watch is terminated (via ctrl-c)?