Skip to content

Routing: run all handlers sequentially and add startup validation #343

Merged
MehakBindra merged 12 commits intonext/corefrom
next/core-routing
Feb 27, 2026
Merged

Routing: run all handlers sequentially and add startup validation #343
MehakBindra merged 12 commits intonext/corefrom
next/core-routing

Conversation

@MehakBindra
Copy link
Collaborator

@MehakBindra MehakBindra commented Feb 23, 2026

Summary

  • All matching routes now run sequentially for non-invoke activities. Previously only the first matching route executed, silently swallowing any subsequent handlers.
    Registration order is preserved.
  • Invoke routing remains first-match-wins. Invoke activities require exactly one InvokeResponse back to Teams — running multiple handlers is undefined behaviour.
  • Startup throws if OnInvoke and specific invoke handlers are mixed. Registering both OnInvoke and eg. OnAdaptiveCardAction guarantees one response is silently swallowed. The valid approaches are: OnInvoke exclusively with an internal if/else, or specific handlers exclusively.
  • Startup throws if the same route is registered twice. Duplicate registration is assumed to be a bug.
  • Removed ILogger from Router. The logger passed was ILogger. Startup errors now throw
    exceptions instead of logging.

@MehakBindra MehakBindra changed the title amend routing Aamend routing: run all handlers sequentially and add startup validation Feb 23, 2026
@MehakBindra MehakBindra changed the title Aamend routing: run all handlers sequentially and add startup validation Amend routing: run all handlers sequentially and add startup validation Feb 23, 2026
@MehakBindra MehakBindra changed the title Amend routing: run all handlers sequentially and add startup validation Routing: run all handlers sequentially and add startup validation Feb 23, 2026
@corinagum
Copy link
Contributor

corinagum commented Feb 23, 2026

First: I think running all matching handlers sequentially is the right call.


I'm leaving this as a non-blocking comment: I'm reallly hesitant about removing the middleware chain. What we're sacrificing by getting rid of next():

  • stopping the chain - handler can determine whether to continue or stop the chain
  • before/after logic - code executed both before and after calling next(), like Express.js middleware wrapping

Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

I'm very hesitant about this one. Will leave some comments in Teams.

Copy link
Collaborator

@singhk97 singhk97 left a comment

Choose a reason for hiding this comment

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

one nit: but looks good to me otherwise

@rido-min rido-min added the CORE label Feb 25, 2026
@MehakBindra MehakBindra enabled auto-merge (squash) February 27, 2026 01:14
auto-merge was automatically disabled February 27, 2026 01:16

Repository rule violations found

@MehakBindra MehakBindra merged commit 576d55c into next/core Feb 27, 2026
3 checks passed
@MehakBindra MehakBindra deleted the next/core-routing branch February 27, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants