Skip to content

Conversation

@adamhenson
Copy link
Collaborator

Summary

I have a theory that this will fix the issue raised in #1. Although it's just a theory, this change is an improvement regardless. The expected outcome described in the issue is that a consumer such as a GitHub Action will stop execution on error. An error from this module should bubble up and kill the process. I'm not certain about the inner workings of GitHub actions and what causes them to terminate, but perhaps in this case the actual outcome is that the Chrome process opened by Chrome Launcher from this module is never closed, because the module throws an exception before executing chrome.kill(). And perhaps because the Chrome process is left open, the GitHub Action never terminates.

@adamhenson adamhenson changed the title fix: ensure we kill chrome fix: ensure the Chrome process terminates on error Apr 29, 2020
@adamhenson
Copy link
Collaborator Author

I'm able to reproduce the issue here https://github.com/foo-software/lighthouse-check-action/runs/628285514?check_suite_focus=true

My plan is to merge this PR, release a prepatch, test it in the GitHub action and then release the patch.

@adamhenson adamhenson merged commit e1c1350 into master Apr 29, 2020
@adamhenson adamhenson deleted the fix/kill-chrome branch April 29, 2020 03:01
@adamhenson
Copy link
Collaborator Author

This fix addressed issue #1 🎉

You can see the expected behavior with this change in https://github.com/foo-software/lighthouse-check-action/runs/628327494?check_suite_focus=true

Instead of hanging forever (like this example of reproducing the issue) the GitHub Action process terminates with the appropriate error message.

lighthouse-check:
 Error: ENOENT: no such file or directory, open '/foo/lighthouse-report-1588130146304.html'
    at Object.openSync (fs.js:440:3)
    at Object.writeFileSync (fs.js:1265:35)
    at _default (/home/runner/work/lighthouse-check-action/lighthouse-check-action/node_modules/@foo-software/lighthouse-persist/dist/index.js:92:19)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async localLighthouse (/home/runner/work/lighthouse-check-action/lighthouse-check-action/node_modules/@foo-software/lighthouse-check/dist/localLighthouse.js:78:7)
    at async _default (/home/runner/work/lighthouse-check-action/lighthouse-check-action/node_modules/@foo-software/lighthouse-check/dist/localLighthouse.js:145:35)
    at async /home/runner/work/lighthouse-check-action/lighthouse-check-action/node_modules/@foo-software/lighthouse-check/dist/lighthouseCheck.js:163:32 {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/foo/lighthouse-report-1588130146304.html'
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants