Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sauceConnectLoader] we should await fs.rename & fs.chmod to avoid race conditions #241

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

naruaway
Copy link
Contributor

@naruaway naruaway commented Apr 12, 2024

[sauceConnectLoader] we should await fs.rename & fs.chmod to avoid race conditions

Description

saucelabs npm package downloads sc binary via verifyAlreadyDownloaded async function before actually invoking it.
However, some of the file system operations are not properly awaited so if they are slow, spawn can happen before making sure sc binary is ready in the correct path.

Note that this type of bugs are easily detected by static checks if we use TypeScript + ESLint (no-floating-promises is part of recommended-type-checked)

Types of Changes

What types of changes does your code introduce? Keep the ones that apply:

  • Bug fix (non-breaking change which fixes an issue)

Tasks

List of tasks you will do to complete the PR

  • Add CHANGELOG

Review

List of tasks the reviewer must do to review the PR

  • Tests
  • Documentation
  • CHANGELOG

Deployment Notes

These should highlight any db migrations, feature toggles, etc.

Nothing to care

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @naruaway!

@diemol diemol merged commit 0f66919 into saucelabs:main Aug 8, 2024
3 checks passed
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