Skip to content

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

Merged
roryabraham merged 5 commits into
mainfrom
Rory-SpawnNotExecSync
Jul 13, 2022
Merged

[No QA] Make GitUtils use spawn/stream instead of execSync/buffer#9891
roryabraham merged 5 commits into
mainfrom
Rory-SpawnNotExecSync

Conversation

@roryabraham

@roryabraham roryabraham commented Jul 13, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

Additional context with the .mjs / await weirdness:

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

@roryabraham roryabraham requested a review from AndrewGable July 13, 2022 22:02
@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
Copy Markdown
Contributor Author

@OSBotify

Copy link
Copy Markdown
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