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: make sonic boom work inside of worker threads #1734

Merged
merged 4 commits into from
Jun 30, 2023
Merged

Conversation

jmcdo29
Copy link
Owner

@jmcdo29 jmcdo29 commented Jun 30, 2023

If we're inside a worker thread, the process.stdout.fd is undefined
which causes a panic in sonic boom and crashes the application.
Defaulting to 1 fixes the issue.

@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2023

🦋 Changeset detected

Latest commit: 5b8dbe3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ogma/logger Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR


stream.on('error', filterBrokenPipe);
// if we are sync: false, we must flush on exit
if (!opts.sync && isMainThread) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Making a note for myself more than anything:

We don't pass the opts.sync option in first place, and child_process does not have an isMainThread property, so this will always evaluate to !false && false i.e. it will always be false. As such, we can remove a good bit of this file and clean things up a bit

If we're inside a worker thread, the process.stdout.fd is undefined
which causes a panic in sonic boom and crashes the application.
Defaulting to `1` fixes the issue.
@nx-cloud
Copy link

nx-cloud bot commented Jun 30, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5b8dbe3. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@jmcdo29 jmcdo29 merged commit 8fd9194 into main Jun 30, 2023
@jmcdo29 jmcdo29 deleted the fix/sonic-boom-opts branch June 30, 2023 16:16
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.

1 participant