Add Middleware handlers to StreamableHttpTransport

[sveneld]

Motivation and Context

This pull request gives a possibility to add different middlewares to StreamableHttpTransport for different approach.

How Has This Been Tested?

Unit tests

Breaking Changes

No

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

[sveneld]

@chr-hertel please take a look

[chr-hertel]

On the first glance, i feel like it makes it more sophisticated, is it needed tho?
The idea would be that frameworks could register their own middleware and solve auth on their own? or would you rework #194 to use that middleware than? or both eventually i guess?

maybe i'm missing the point here ...

[sveneld]

Yep, that’s the idea. #194 will be rewritten on top of this middleware approach, and it also opens the door for community/custom middlewares without patching core.

[sveneld]

@chr-hertel #221 is a pull request with oAuth implementation based on middleware. I created a new pull request instead of update previous

[sveneld]

@chr-hertel maybe we should go step by step and this pull request will be first part for oauth implementing?

[chr-hertel]

In the beginning I was a bit conflicted about bringing in this extension point, bc it felt like we're just doing it for that one case, but it's fair to assume there might be more - and yes, this would be the first step, auth second.

WDYT @Nyholm @CodeWithKyrian @soyuka? (see #221 for more context)

[Nyholm]

I like the idea.

It will help making the SDK more extendable. I have a few small suggestions I want you to address.

Good work!

[Nyholm]

Thank you. I like this PR.

I would like us to consider the stack trace. If I add 14 middleware, then I go in to StreamableHttpTransport::handleRequest(), would I like to see these 14 middleware + 14 MiddlewareRequestHandler in my stack trace?

(Im thinking out loud)

What are the pros and cons for creating a RequestHandler looking something like

class MiddlewareRequestHandler implements RequestHandlerInterface
{
    /**
     * @param MiddlewareInterface[] $middleware
     */
    public function __construct(
        private array $middleware,
        private \Closure $application,
    ) {
    }

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        $middleware = array_shift($this->middleware);
        if (null === $middleware) {
            return ($this->application)($request);
        }

        return $middleware->process($request, $this);
    }
}

With this only "active" middleware is in the stack. And there is no extra RequestHandler.

[Nyholm]

Can you move the 4 lines of logic here to the listen() method?

[sveneld]

in this case i should make createRequestHandler protected. Is it ok?

[Nyholm]

Im not sure what you mean.

I suggest to update the listen method like:

 public function listen(): ResponseInterface
    {
        $handler = new MiddlewareRequestHandler(
            $this->middleware,
            \Closure::fromCallable([$this, 'handleRequest']),
        );;

        return $this->withCorsHeaders($handler->handle($this->request));
    }

[sveneld]

updated

[sveneld]

I would like us to consider the stack trace. If I add 14 middleware, then I go in to StreamableHttpTransport::handleRequest(), would I like to see these 14 middleware + 14 MiddlewareRequestHandler in my stack trace?

Yes, the stack trace will contain 14+ middleware, because they are called one by one.

What are the pros and cons for creating a RequestHandler looking something like
Thank for idea.

I updated a pull request and made one commit.