Skip to content

Commit abbf6b2

Browse files
heyitsaamirclaudeCopilot
authored
Fix ExpressAdapter to accept Express app directly (#504)
## 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 - [x] Existing `express-adapter` tests pass (9/9) - [x] New test: passing an Express app works for both custom routes and adapter-registered routes - [x] Manual: verify Express example compiles and runs correctly --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c2be582 commit abbf6b2

4 files changed

Lines changed: 73 additions & 15 deletions

File tree

examples/http-adapters/express/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import http from 'http';
12
import 'dotenv/config';
2-
import { app, httpServer } from './teams-app';
3+
import { app, expressApp } from './teams-app';
34

45
const port = parseInt(process.env.PORT || '3978', 10);
56

@@ -9,7 +10,8 @@ async function main() {
910
// Initialize teams.ts app - this adds /api/messages to your Express app
1011
await app.initialize();
1112

12-
// Start your Express server
13+
// Start your server — you control the lifecycle
14+
const httpServer = http.createServer(expressApp);
1315
await new Promise<void>((resolve, reject) => {
1416
httpServer.listen(port, () => resolve());
1517
httpServer.once('error', reject);

examples/http-adapters/express/teams-app.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import http from 'http';
21
import express from 'express';
32
import { App, ExpressAdapter } from '@microsoft/teams.apps';
43

54
// 1. Create your existing Express app with routes
65
export const expressApp = express();
7-
export const httpServer = http.createServer(expressApp);
86

97
// Add your custom routes
108
expressApp.get('/health', (_req, res) => {
@@ -36,8 +34,8 @@ expressApp.get('/', (_req, res) => {
3634
`);
3735
});
3836

39-
// 2. Create Express adapter with your existing server
40-
export const adapter = new ExpressAdapter(httpServer);
37+
// 2. Create Express adapter with your existing Express app
38+
export const adapter = new ExpressAdapter(expressApp);
4139

4240
// 3. Create teams.ts app with the adapter
4341
export const app = new App({

packages/apps/src/http/express-adapter.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import http from 'http';
44
import os from 'os';
55
import path from 'path';
66

7+
import express from 'express';
78
import supertest from 'supertest';
89

910
import { ExpressAdapter } from './express-adapter';
@@ -25,6 +26,41 @@ describe('ExpressAdapter', () => {
2526
}).not.toThrow();
2627
});
2728

29+
it('should accept an Express app and handle requests without double handling', async () => {
30+
const app = express();
31+
app.get('/health', (_req, res) => {
32+
res.json({ ok: true });
33+
});
34+
35+
adapter = new ExpressAdapter(app);
36+
37+
adapter.registerRoute('POST', '/api/messages', async ({ body }) => {
38+
return { status: 200, body: { echo: (body as Record<string, unknown>).message } };
39+
});
40+
41+
// User manages the server themselves
42+
server = http.createServer(app);
43+
44+
// Verify custom routes on the Express app still work
45+
const healthRes = await supertest(server).get('/health').expect(200);
46+
expect(healthRes.body).toEqual({ ok: true });
47+
48+
// Verify teams route registered by the adapter works
49+
const botRes = await supertest(server)
50+
.post('/api/messages')
51+
.send({ message: 'hello' })
52+
.expect(200);
53+
expect(botRes.body).toEqual({ echo: 'hello' });
54+
});
55+
56+
it('should throw on start/stop when Express app is passed', async () => {
57+
const app = express();
58+
adapter = new ExpressAdapter(app);
59+
60+
await expect(adapter.start(3000)).rejects.toThrow('server lifecycle is managed externally');
61+
await expect(adapter.stop()).rejects.toThrow('server lifecycle is managed externally');
62+
});
63+
2864
describe('route registration and request handling', () => {
2965
it('should handle POST requests with JSON body parsing', async () => {
3066
server = http.createServer();

packages/apps/src/http/express-adapter.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,24 @@ export class ExpressAdapter implements IHttpServerAdapter {
2727
readonly use: express.Application['use'];
2828

2929
protected express: express.Application;
30-
protected server: http.Server;
30+
protected server?: http.Server;
3131
protected logger: ILogger;
3232
protected onError?: (err: Error) => void;
3333

34-
constructor(server?: http.Server, options?: { logger?: ILogger; onError?: (err: Error) => void }) {
35-
this.express = express();
36-
this.server = server || http.createServer();
37-
this.server.on('request', this.express);
34+
constructor(serverOrApp?: http.Server | express.Application, options?: { logger?: ILogger; onError?: (err: Error) => void }) {
35+
if (serverOrApp instanceof http.Server) {
36+
// The adapter handles all requests on this server. Use the adapter's
37+
// methods (get, post, use, etc.) to add routes. If you need your own
38+
// Express app, pass it in instead and manage the server yourself.
39+
this.express = express();
40+
this.server = serverOrApp;
41+
this.server.on('request', this.express);
42+
} else if (typeof serverOrApp === 'function') {
43+
this.express = serverOrApp;
44+
} else {
45+
this.express = express();
46+
this.server = http.createServer(this.express);
47+
}
3848
this.logger = options?.logger ?? new ConsoleLogger('ExpressAdapter');
3949
this.onError = options?.onError;
4050

@@ -79,21 +89,27 @@ export class ExpressAdapter implements IHttpServerAdapter {
7989
* Start the server listening on the specified port
8090
*/
8191
async start(port: number | string): Promise<void> {
92+
const server = this.server;
93+
94+
if (!server) {
95+
throw new Error('Cannot start: server lifecycle is managed externally. Call listen() on your Express app or http.Server directly.');
96+
}
97+
8298
return new Promise<void>((resolve, reject) => {
8399
// Handle startup errors
84-
this.server.once('error', (err) => {
100+
server.once('error', (err) => {
85101
if (this.onError) {
86102
this.onError(err);
87103
}
88104
reject(err);
89105
});
90106

91-
this.server.listen(port, () => {
107+
server.listen(port, () => {
92108
this.logger.info(`listening on port ${port} 🚀`);
93109

94110
// Set up persistent error listener after startup
95111
if (this.onError) {
96-
this.server.on('error', this.onError);
112+
server.on('error', this.onError);
97113
}
98114

99115
resolve();
@@ -112,8 +128,14 @@ export class ExpressAdapter implements IHttpServerAdapter {
112128
* Stop the server and close all connections
113129
*/
114130
async stop(): Promise<void> {
131+
const server = this.server;
132+
133+
if (!server) {
134+
throw new Error('Cannot stop: server lifecycle is managed externally. Call close() on your Express app or http.Server directly.');
135+
}
136+
115137
return new Promise<void>((resolve, reject) => {
116-
this.server.close((err) => {
138+
server.close((err) => {
117139
if (err) {
118140
reject(err);
119141
} else {

0 commit comments

Comments
 (0)