-
Notifications
You must be signed in to change notification settings - Fork 25
Introduce HttpServer (and begin deprecating HttpPlugin) #433
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
base: aamirj/simlifyHttp
Are you sure you want to change the base?
Conversation
f07e077 to
d9a529e
Compare
6601a42 to
7e55c15
Compare
examples/http-adapters/README.md
Outdated
|
|
||
| An HTTP adapter bridges a specific HTTP framework (Express, Hono, Next.js, etc.) with teams.ts. This allows you to: | ||
|
|
||
| - ✅ Use any HTTP framework you prefer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like how we showcase different adapters
examples/http-adapters/README.md
Outdated
|
|
||
| All adapters start on `http://localhost:3978` with the Teams bot endpoint at `/api/messages`. | ||
|
|
||
| ## Architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be nice to capture this in our official docs later :)
| ); | ||
| } | ||
| await this._adapter.start(portNumber); | ||
| this.logger.info(`listening on port ${port} 🚀`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is most likely never going to hit.
examples/http-adapters/package.json
Outdated
| "@microsoft/teams.api": "2.0.5", | ||
| "@microsoft/teams.apps": "2.0.5", | ||
| "@microsoft/teams.common": "2.0.5", | ||
| "express": "^4.21.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we update to express 5?
| }); | ||
|
|
||
| expressApp.get('/', (req, res) => { | ||
| res.send(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is cute, should we do the same in other languages/samples?
2fa2032 to
4590e50
Compare
64d7196 to
4c1ca04
Compare
The previous architecture tightly coupled HTTP transport concerns with activity sending logic:
**Previous Architecture:**
```
HttpPlugin (transport) → implements ISender (sending)
→ has send() method (creates new Client per call)
→ has createStream() method
→ knows about Activity protocol details
ActivityContext → depends on ISender plugin
→ cannot work without transport plugin
→ conflates transport and sending concerns
```
Key Issues:
- HttpPlugin created NEW Client instances on every send() call
- Transport plugins (HttpPlugin) were forced to implement send/createStream
- Users couldn't "bring their own server" without implementing ISender
- ActivityContext was tightly coupled to plugin architecture
- HttpPlugin knew about Bot Framework Activity protocol details
```
HttpPlugin (transport) → only handles HTTP server/routing/auth
→ emits ICoreActivity (minimal protocol knowledge)
→ just passes body payload to app
ActivitySender (NEW) → dedicated class for sending activities
→ receives injected, reusable Client
→ handles all send/stream logic
→ private to App class
ActivityContext → uses send callback (abstraction)
→ receives pre-created stream
→ no direct dependency on ActivitySender
```
- Centralized all activity sending logic
- Receives reusable Client in constructor (no per-send instantiation)
- Private to App class - internal implementation detail
- Provides send() and createStream() methods
- Minimal fields transport layer needs: serviceUrl, id, type
- Extensible via [key: string]: any for protocol-specific fields
- Transport plugins work with this instead of full Activity type
- Parsing to Activity happens in app.process.ts
- No longer needed - plugins don't send activities
- Plugins only handle transport (receiving requests)
- Breaking change, but simplifies plugin architecture
- Constructor accepts send callback function
- Receives pre-created stream (not factory function)
- No knowledge of ActivitySender implementation
- Proper abstraction via dependency injection
- Initially renamed to ActivityStream
- Reverted because it's still HTTP-specific (uses Bot Framework HTTP Client API)
- Moved from src/plugins/http/stream.ts to src/http-stream.ts
- Still transport-specific, just not plugin-owned
1. **ISender removed** - Custom plugins should implement IPlugin only
2. **IActivityEvent changed** - Now has body: ICoreActivity instead of activity: Activity
3. **Plugin.onActivity** - Still receives parsed activity: Activity (unchanged)
4. **App.process signature** - Internal change, not exposed to plugin API
Before:
```typescript
class MyPlugin implements ISender {
send(activity, ref) { ... } // Required
createStream(ref) { ... } // Required
async onRequest(req, res) {
const activity: Activity = req.body; // Need Activity type
await this.$onActivity({ activity, token });
}
}
```
After:
```typescript
class MyPlugin implements IPlugin {
// No send() or createStream() needed!
async onRequest(req, res) {
// Just pass body - don't need Activity type knowledge
await this.$onActivity({
body: req.body, // ICoreActivity (minimal fields)
token
});
}
}
```
1. **Client Reuse** - ActivitySender reuses same Client, no per-send instantiation
2. **Separation of Concerns** - Transport vs sending clearly separated
3. **Bring Your Own Server** - Easy to implement custom transports (Socket.io, gRPC, etc.)
4. **Less Protocol Knowledge** - Transport layer only needs ICoreActivity, not full Activity
5. **Cleaner Architecture** - Each class has single responsibility
6. **Better Abstraction** - ActivityContext uses callbacks, not direct dependencies
- src/activity-sender.ts - Dedicated activity sending class
- src/http-stream.ts - Moved from src/plugins/http/stream.ts
- src/app.ts - Added activitySender, updated send()
- src/app.process.ts - Removed sender param, uses activitySender
- src/plugins/http/plugin.ts - Removed send/createStream, works with ICoreActivity
- src/contexts/activity.ts - Uses callbacks instead of ISender plugin
- src/events/activity.ts - Added ICoreActivity, changed to body field
- src/types/plugin/sender.ts - Removed ISender, kept IActivitySender
- src/plugins/http/stream.ts - Moved to src/http-stream.ts
- ISender interface - Completely removed
4c1ca04 to
5422e29
Compare
In this PR, we introduce a new object called HttpServer and begin to deprecate HttpPlugin.
Why:
HTTP is a core part of our sdk. Our App object uses HTTP to set up a server, perform auth validations, and pipe the request to the handlers that are attached, and then return the response. Key part is that Http is a core part of App, not a plugin, since core functionality is dependent on it.
Even inside the App object, we were doing special casing for this Http"Plugin" whereas it should never have really been a plugin to begin with. By making it a plugin, we were exposing many non-plugin essential things to the plugin system in general.
So what should it have been? Well, HTTP Plugin had these responsibilities
So, we introduce a new object called
HttpServerwhose responsibilities are essentially that ^. This object is not a plugin, but an object that's created by App itself.Customization
Now this idealogical shift doesn't really warrant us doing this refactor, but we started seeing requests from folks who wanted to hook Teams functionality into existing servers, or replace the underlying server infra with a non-express server. Our recommendation was to rebuild a new HttpPlugin. But rebuilding this plugin is not simple (since we don't really document it anywhere, and didn't expect folks to build their own).
So
HttpServerexposes anHttpAdapterconcept. To build the adapter, one simply needs to build out a handler for extracting request data, and a handler for responses. This means that you can build simple custom adapters for your own existing servers. (And if you don't pass one in, we'll build a default express one.) Examples of servers are in the http-adapters folder under examples/.Backward Compat
We've updated
HttpPluginto basically useHttpServerwith anExpressAdapterinternally for backward compat. I don't think this should lead to any breaking changes (even if someone passes in their ownHttpPlugin). (Tested BotBuilderPlugin, from examples, and it worked without any changes).However, it should be noted that I marked HttpPlugin as deprecated in this PR, so it should be discouraged going forward, and after the next few versions, it'll be removed.
PR Dependency Tree
This tree was auto-generated by Charcoal