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

[No QA] Make GitUtils use spawn/stream instead of execSync/buffer #9891

Merged
merged 5 commits into from
Jul 13, 2022

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jul 13, 2022

Details

This fixes an error we've never seen before with our deploy process: https://expensify.slack.com/archives/C03V9A4TB/p1657736346352819

The problem is that execSync uses a buffer with a 200kb limit, and if the command's output exceeds that amount then it will 🤮

This solution uses spawn, which uses an output stream instead of a buffer, so we don't have to worry about running out of buffer space. Got the solution from https://stackoverflow.com/questions/63796633/spawnsync-bin-sh-enobufs

Alternate solution: Use increase the buffer size. But then there's no telling if we'd run into this problem in the future if we had a larger commit volume / more time between deploys.

Fixed Issues

$ n/a – broken deploys

Tests

  1. Automated tests using this function are updated and passing.
  2. Merge this PR
  3. Verify that it triggers a deploy.

@roryabraham roryabraham self-assigned this Jul 13, 2022
@roryabraham roryabraham changed the title Make GitUtils use spawn/stream instead of execSync/buffer [No QA] Make GitUtils use spawn/stream instead of execSync/buffer Jul 13, 2022
@roryabraham roryabraham marked this pull request as ready for review July 13, 2022 22:00
@roryabraham roryabraham requested a review from a team as a code owner July 13, 2022 22:00
@melvin-bot melvin-bot bot requested review from puneetlath and removed request for a team July 13, 2022 22:00
@roryabraham
Copy link
Contributor Author

Additional context with the .mjs / await weirdness:

https://expensify.slack.com/archives/C01GTK53T8Q/p1657745375254589

@roryabraham roryabraham merged commit b1a16fc into main Jul 13, 2022
@roryabraham roryabraham deleted the Rory-SpawnNotExecSync branch July 13, 2022 22:48
@roryabraham
Copy link
Contributor Author

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @chiragsalian in version: 1.1.84-13 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

3 participants