Skip to content

Fix ExpressAdapter to accept Express app directly#504

Merged
heyitsaamir merged 8 commits into
mainfrom
fix/express-adapter-accept-app
Apr 6, 2026
Merged

Fix ExpressAdapter to accept Express app directly#504
heyitsaamir merged 8 commits into
mainfrom
fix/express-adapter-accept-app

Conversation

@heyitsaamir

@heyitsaamir heyitsaamir commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix ERR_HTTP_HEADERS_SENT when passing an http.Server that already has an Express app attached — the adapter no longer creates a second Express app in that scenario
  • ExpressAdapter constructor now accepts http.Server | express.Application | undefined:
    • Express app passed: uses it directly - assumes the http server is being managed by the user directly.
    • http.Server passed: creates internal Express app, attaches to server (existing behavior)
    • Nothing passed: creates both (existing behavior)
  • Updated Express example to pass the Express app directly (the recommended pattern)

The original http.Server-only constructor was designed to match the HttpPlugin's capabilities, but it led users into a trap: passing http.createServer(app) caused the adapter to attach a second Express app to the same server, resulting in double request handling. Accepting the Express app directly is the natural fix. Ideally, we should probably just not accept an http server and just accept an existing express app. If someone is managing an express app outside, they can continue doing that, and they should be responsible for handling the http server too.

Fixes #503

Test plan

  • Existing express-adapter tests pass (9/9)
  • New test: passing an Express app works for both custom routes and adapter-registered routes
  • Manual: verify Express example compiles and runs correctly

heyitsaamir and others added 3 commits March 24, 2026 23:48
These two markdown files are all a human needs to write — the e2e testing
skill generates and manages the actual Playwright test code from here.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
When users passed an http.Server that already had an Express app
(e.g. http.createServer(app)), the adapter created a second Express
app and attached it via server.on('request'), causing double request
handling and ERR_HTTP_HEADERS_SENT errors.

The constructor now accepts http.Server | express.Application:
- Express app: uses it directly, creates server from it (no double handling)
- http.Server: creates internal Express app, attaches to server
- Nothing: creates both (existing behavior)

Fixes #503

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@heyitsaamir
heyitsaamir marked this pull request as ready for review April 4, 2026 17:43
@heyitsaamir
heyitsaamir requested a review from Copilot April 6, 2026 16:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the ExpressAdapter to accept an express.Application directly (in addition to an http.Server) to avoid accidentally attaching two Express apps to the same Node server, which can lead to ERR_HTTP_HEADERS_SENT. It also updates the Express http-adapter example and adds tests for the new constructor behavior.

Changes:

  • Extend ExpressAdapter constructor to accept http.Server | express.Application | undefined and adjust lifecycle handling.
  • Add tests for passing an Express app into ExpressAdapter and for start/stop behavior when lifecycle is external.
  • Update the Express http-adapter example to pass an Express app into the adapter and create the server externally.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
packages/apps/src/http/express-adapter.ts Adds support for passing an Express app and changes server lifecycle behavior.
packages/apps/src/http/express-adapter.spec.ts Adds coverage for Express-app construction and lifecycle errors.
examples/http-adapters/express/teams-app.ts Updates example to construct adapter with the Express app instead of an http.Server.
examples/http-adapters/express/index.ts Moves server creation/listen into the example’s entrypoint to reflect external lifecycle management.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/apps/src/http/express-adapter.ts
Comment thread packages/apps/src/http/express-adapter.ts
Comment thread packages/apps/src/http/express-adapter.spec.ts Outdated
Comment thread packages/apps/src/http/express-adapter.ts

@corinagum corinagum left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm! - tiny nit

@heyitsaamir
heyitsaamir merged commit abbf6b2 into main Apr 6, 2026
10 checks passed
@heyitsaamir
heyitsaamir deleted the fix/express-adapter-accept-app branch April 6, 2026 22: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.

[Bug]: ExpressAdapter throws headers already sent error

3 participants