actions / toolkit

The GitHub ToolKit for developing GitHub Actions.
https://github.com/features/actions
MIT License
4.95k stars 1.42k forks source link

Introduce the ability to interrupt exec() using signal (PR: #1469) #1534

Open Fgerthoffert opened 1 year ago

Fgerthoffert commented 1 year ago

I initially created a PR right away considering the low complexity of the change but I'm not sure if it went under the radar or not.

https://github.com/actions/toolkit/pull/1469

Describe the enhancement When creating js/ts actions, it's currently not possible to implement a timeout mechanism within only a portion of an action.

The NodeJS child.spaws() API used by the toolkit/action does support the signal mechanism.

The goal here is to make it possible to use signal in the action.

Code Snippet

try {
  core.info(`Command starting at: ${JSON.stringify(new Date())}`)
  await exec.exec("sleep 10000", [], {signal: AbortSignal.timeout(5000)})
  core.info(`Command completed at: ${JSON.stringify(new Date())}`)
} catch (error: any) {
  if (error.name === "AbortError") {
    core.info(`Timeout reached at: ${JSON.stringify(new Date())}. The command was interrupted`)
  } else {
    core.info(`There was an issue processing the command (${error.name}). It failed at: ${JSON.stringify(new Date())}`)
  }
}
breathe commented 11 months ago

I would like similar functionality -- though I'm not clear that what I need exactly matches above.

I would like to add graceful shutdown support to the terraform-wrapper https://github.com/hashicorp/setup-terraform/blob/1b93182764c8332e7679b2393cb307cbe7baf9dc/wrapper/terraform.js#L39 -- which currently blows up when calling task is cancelled. I believe that it could be made to work better if:

(1) the wrapper forwarded signals to the terraform process (2) the caller of terraform bypasses bash (exec terraform apply ... vs terraform apply)

Fgerthoffert commented 7 months ago

We're still hoping for this Issue and its associated PR (https://github.com/actions/toolkit/pull/1469) to get some attention/review, not being able to interrupt an exec() in Typescript actions is a pain if the said action must perform other tasks after a potential timeout.

The fix is trivial, already documented as a possible option in nodeJS, and is not expected to have an adverse impact.