Skip to content

fix: settle startup failures and clean up owned server resources - #5727

Open
OskarEichler wants to merge 5 commits into
webpack:mainfrom
OskarEichler:codex/server-startup-cleanup
Open

fix: settle startup failures and clean up owned server resources#5727
OskarEichler wants to merge 5 commits into
webpack:mainfrom
OskarEichler:codex/server-startup-cleanup

Conversation

@OskarEichler

@OskarEichler OskarEichler commented Aug 28, 2026

Copy link
Copy Markdown

Fixes

  • Reject occupied IPC/TCP startup through the async API instead of throwing outside it and leaving the promise pending. Close the IPC probe.
  • Attach an HTTP upgrade handler only in noServer mode. Remove only this implementation's handler during close; avoid duplicate handling for an existing HTTP server and needless handling for a separate WebSocket port.
  • Wait for pending plugin setup/listen before stopping, and do not begin listening after shutdown has started.
  • Fix Bonjour protocol logging precedence.

Compatibility

No API/engine/dependency change. Startup errors now reach awaited start()/callback error handling; callers must handle rejection. Shutdown may wait for already-started initialization before resolving. Unrelated HTTP listeners remain untouched. Bonjour log text reflects explicit protocol selection.

Verification

Real loopback HTTP/WebSocket checks cover owned listener registration/removal, shared/separate server modes and connections. Bounded occupied TCP/IPC controls verify promise rejection without uncaught exceptions. Real webpack compiler hooks and a real pending HTTP listen verify startup/shutdown ordering and closed sockets. All 17 existing plugin API tests pass in the frozen full run. Combined server controls: 14 pass; middleware/reporting/startup controls: 15 pass. Build and all lint/type checks pass.

Audit scope

This is a focused, independently based change from a broader source review at f804962. The combined frozen-source run executed 1,002 tests: 951 passed, 43 failed, one cancelled, seven skipped. Failures were traced to the separately proposed overlay DOM snapshots, reconnect-disabled test expectation and IPv6 host-test assumptions; it is not represented as a green full suite. Relevant focused results are listed above. No checked-in tests/specs/snapshots were added or modified. Hosted CI and the full OS/Node matrix remain pending.

Summary by CodeRabbit

  • Bug Fixes
    • Startup now fails cleanly when the configured TCP port or IPC path is already in use.
    • WebSocket upgrade listeners are cleaned up correctly when the server closes.
    • Shutdown waits for pending startup operations and plugin initialization to finish.
    • Bonjour status now accurately reports the configured HTTP or HTTPS protocol.
    • Server startup errors are reported through rejected promises instead of uncaught exceptions.

@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 78371cc

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

This PR includes changesets to release 1 package
Name Type
webpack-dev-server 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

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 67829fa1-cc31-4b6f-9880-26a78b64074a

📥 Commits

Reviewing files that changed from the base of the PR and between e5ea9d3 and 78371cc.

📒 Files selected for processing (2)
  • lib/Server.js
  • test/e2e/api.test.js

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.


Walkthrough

The change updates webpack-dev-server startup and shutdown behavior. Occupied TCP and IPC endpoints now reject startup promises. Failed starts stop initialized resources. Plugin mode tracks pending setup and listen operations during shutdown. WebSocket upgrade listeners are registered only in noServer mode and removed on close. Bonjour status reporting preserves a configured protocol. A patch changeset and API tests document and verify these updates.

Merge Risk: 🟡 Moderate · up to 78371

The PR improves startup error handling and resource cleanup, but direct startup and shutdown can still race so a listener may become active after shutdown reports completion, while some failed setup paths may not clean up fully and plugin bind failures may not retry. This is a bounded lifecycle correctness risk that should be explicitly reviewed or addressed before merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: resolving startup failures and cleaning up server resources.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 35f010e7-3a6a-46a2-a936-6042883334c7

📥 Commits

Reviewing files that changed from the base of the PR and between f804962 and b398d32.

📒 Files selected for processing (3)
  • .changeset/server-startup-cleanup.md
  • lib/Server.js
  • lib/servers/WebsocketServer.js

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread lib/Server.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e4d39fa-1683-45cb-a9ad-3187eb2b9e63

📥 Commits

Reviewing files that changed from the base of the PR and between b398d32 and e5ea9d3.

📒 Files selected for processing (2)
  • lib/Server.js
  • test/e2e/api.test.js

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Comment thread lib/Server.js
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