hendrikmuhs / ccache-action

github action to speedup building using ccache
MIT License
122 stars 54 forks source link

fix: early exit process so node doesn't wait for hanging promises #182

Closed chirag-droid closed 7 months ago

chirag-droid commented 9 months ago

Issue

fixes: #181

Proposed Changes

Since Node waits for hanging http requests we early exit by calling process.exit. This doesn't affect the existing API but we can specify optional parameter earlyExit that would apply this fix.

This is related to https://github.com/nodejs/node/issues/47228

chirag-droid commented 9 months ago

oh my... i misspelled wait to want

echoix commented 7 months ago

Would it be possible to reconsider merging this PR? Rebasing the changes made on main before?

I'm trying to add this action to save a bit of time in CI, but I'm loosing more time in the post step than the little time saved + overhead

hendrikmuhs commented 7 months ago

As said in the issue: I happily merge it once the test-failures are resolved. If @chirag-droid lost interest, maybe you can take over?

chirag-droid commented 7 months ago

As said in the issue: I happily merge it once the test-failures are resolved. If @chirag-droid lost interest, maybe you can take over?

I can take up this issue again. I am diagnosing the failed tests

chirag-droid commented 7 months ago

image

https://github.com/hendrikmuhs/ccache-action/actions/runs/7697481224/job/22066139179?pr=182

I need help with this test case. And why it's failing.

mweberxyz commented 7 months ago

@chirag-droid not sure about that exact failure, but process exit codes should be positive numbers. 0 means success, non-0 for failure. Generally, process.exit(1) is what you are looking for in the error handlers.

hendrikmuhs commented 7 months ago

Thank you @chirag-droid. The 1 failing test doesn't matter. I can fix that manually. I will merge this PR in the afternoon.

junaire commented 7 months ago

Do we need to make a new release? @hendrikmuhs

Naros commented 7 months ago

Is there an update on when we can expect this to be released @hendrikmuhs ?

njzjz commented 7 months ago

This is a workaround for me before a new version is released:

    - uses: hendrikmuhs/ccache-action@ff9f6cc67d49b9acaa9e102f80d1ea8a0c86a6dd
hendrikmuhs commented 7 months ago

sorry, didn't had time to make a release. Will do it now.