cache
cache copied to clipboard
Exit after run to prevent hanging if there are active listeners
Closes https://github.com/actions/cache/issues/370
If there are active listeners after run()
finishes, NodeJS will not exit. Generally, leaving an active listener is a bug that should be fixed, but to prevent this from causing the step to hang we can explicitly exit.
Could you double-check the generated dist files? I wouldn't expect 100k lines to be changed for those.
Whoops, this should be process.exit()
so that we honor any process.exitCode
previously set.
Ah, as @ericsciple pointed out, we need to flush stdout/stderr, so just calling process.exit()
isn't safe.
Another option would be to set a timeout for writing stdout/stderr and then force exit:
const OUTPUT_TIMEOUT = 60_000
async function main() {
// My action code
}
runSafeMain(main)
function runSafeMain(fn: (...args: any[]) => Promise<unknown>) {
fn().finally(waitForOutput)
}
function waitForOutput() {
const timeout = setTimeout(() => {
process.exit()
}, OUTPUT_TIMEOUT)
// Ensure the timeout doesn't block the process from exiting normally
timeout.unref()
}
See also this technique discussed in joyent/node-exuent.
@joshmgross Fixed the dist/
files having tons of changes. Had it pointing to a local copy of @actions/toolkit
. (The test that looks for uncommitted changes also detected this issue).
I'll wait until I hear back from @jclem
and @ericsciple
on how to handle flushing before doing anything with this.
[warning]downloadCache failed: Request timeout: /758370072ebc48a1bae6dd969702511b/59eed5cca5c5ea119b0600155dafe08a?sv=2018-03-28&sr=b&sig=4O4ae0FEDWtyXHalSOBL7FMQNrPJdJSajOxwzi2WJsY%3D&se=2020-07-14T11%3A18%3A50Z&sp=r&rscl=x-e2eid-ca91503f-2fdf483a-86d18754-8bf30f0c
8
##[error]The operation was canceled.
It seems from the log the request got stuck after the timeout error. I suspect it's because of a bug in the http-client package we use (code here) but I'm not proficient enough in nodejs runtime to figure out where.. @jclem @joshmgross Any idea?
@dhadka I'm changing default branch to main
for cache so I rebased your PR to on top of main
@vsvipul are you reactivating this PR?
@bishal-pdMSFT I'm not sure if this is still required. I'll check and follow up accordingly.
Closing this for now, as we aren't facing any issues related to this.