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

fix: concurrent api builds #11109

Merged
merged 3 commits into from
Jul 29, 2024
Merged

fix: concurrent api builds #11109

merged 3 commits into from
Jul 29, 2024

Conversation

callingmedic911
Copy link
Member

@callingmedic911 callingmedic911 commented Jul 28, 2024

A few users reported that the API server crashes with the error EADDRINUSE when switching between branches. This issue happens on the API side when:

  1. New files are added or existing files are removed.
  2. Immediately after, an existing file is changed.

This scenario is common when doing git operations like switching branches or using git stash, where these changes occur simultaneously. When this happens, step 1 triggers a full build (without esbuild's rebuild), and step 2, without canceling the build from step 1, triggers a separate rebuild. This results in concurrent builds and two instances of the API server trying to start.

This PR provides a quick fix for the issue. It's not ideal to cancel a full build (without a rebuild flag) with a rebuild. I created a follow-up PR to address this + refactor the process a bit.

@dac09
Copy link
Contributor

dac09 commented Jul 29, 2024

Trust your thinking on this @callingmedic911 - and don't see any harm in cancelling an existing build if its running.

@dac09 dac09 enabled auto-merge (squash) July 29, 2024 05:40
@dac09 dac09 merged commit 5c61ee0 into main Jul 29, 2024
46 checks passed
@dac09 dac09 deleted the aditya/fix-concurrent-api-builds branch July 29, 2024 05:49
callingmedic911 added a commit that referenced this pull request Aug 9, 2024
Follow-up to #11109.

While working on #11109, I started making changes that turned into a
larger refactor, that's why this separate PR:
- **Fix:** Centralized debounce logic with precedence of flags (e.g., if
a `rebuild: false` call happens while debounced, it takes precedence,
even if subsequent debounced calls were with `true`).
- Decoupled the build and HTTP server logic in `watch.ts`. The main
motivation was to test parts of the build process I modified. This also
improves readability with clearer responsibilities.
- Added tests for the debounce with precedence logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants