fix: forward signals to child process in npm launcher#347
Open
gtsiolis wants to merge 1 commit into
Open
Conversation
gtsiolis
added
semver: patch
docs: skip
gtsiolis
force-pushed
the
devx-942-node-wrapper-does-not-forward-signals-to-child-process-b667
branch
from
6f2d272 to
f3adc03
Compare
The npm wrapper auto-generated by goreleaser-npm-publisher spawned the Go binary without installing any signal handlers. A programmatic `kill <lstk-pid>` terminated the Node wrapper but left the Go binary running, orphaning mid-flight container starts. Interactive Ctrl-C worked only because the TTY signals the whole process group. Ship our own launcher (npm/launcher.js) that resolves the platform binary, forwards SIGINT/SIGTERM/SIGHUP to the child, and propagates its exit code / terminating signal. The release job now builds the npm packages, swaps in this launcher, and publishes — replacing the single evg4b/goreleaser-npm-publisher-action step. A node --test job covers the forwarding behaviour. Generated with [Linear](https://linear.app/localstack/issue/DEVX-942/node-wrapper-does-not-forward-signals-to-child-process#agent-session-91e363a5) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
gtsiolis
force-pushed
the
devx-942-node-wrapper-does-not-forward-signals-to-child-process-b667
branch
from
f3adc03 to
2ad4b7d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
@localstack/lstknpm wrapper was auto-generated bygoreleaser-npm-publisherand spawned the Go binary with no signal handlers. A programmatickill <lstk-pid>killed the Node wrapper but left the Go binary running, orphaning mid-flight container starts. Interactive Ctrl-C only worked because the TTY signals the whole process group.This ships our own launcher (
npm/launcher.js) that resolves the platform binary from the installed optional dependency, forwardsSIGINT/SIGTERM/SIGHUPto the child, and propagates the child's exit code / terminating signal.Since that wrapper isn't customizable through the publishing action, the release job now runs
goreleaser-npm-publisher buildexplicitly, overwrites the generateddist/npm/lstk/index.jswith our launcher, thennpm publishes each package — replacing the singleevg4b/goreleaser-npm-publisher-actionstep. Platform packages are byte-identical to before; only the main package's wrapper changes.A new
test-launcherCI job runsnode --test npm/*.test.js; the regression test spawns the launcher against a fake binary, sendsSIGTERM, and asserts the child receives it and its exit code propagates out.Note
The publish pipeline change only runs on tagged releases, so it can't be exercised by PR CI — worth a close look from a reviewer.