go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

Proposal: add StreamableHTTPHandler.Close

Open findleyr opened this issue 4 months ago • 2 comments

While writing a test for #265, it occurred to me that we probably should expose a StreamableHTTPHandler.Close. Otherwise, there's no way to close the handler while also guaranteeing that no new sessions are subsequently added.

Specifically, I propose the following:

// Close closes the handler, closing and removing all connected sessions,
// and preventing new sessions from being added.
func (*StreamableHTTPHandler) Close() error

I think we didn't do this initially because there was a thought that with #148, perhaps we wouldn't need it if the handler was effectively stateless and only interacted with a session store.

However, I don't think there's much harm in adding Close. Even if we eventually accept a session store, close may be useful.

This is not urgent.

findleyr avatar Sep 09 '25 20:09 findleyr

Hi @findleyr, are we considering this before v1.0.0? I can take this up if not planned? by you.

This will allow for cleaning resources using defer.

func foo() {
	router := http.NewServeMux()

	handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server {
		return server
	}, nil)
	defer handler.Close() // this line
	
	router.Handle("/mcp", handler)
	http.ListenAndServe(":8080", router)
}

With this we can also replace closeAll method with more idiomatic way.

Implementation could look like

// Close closes the handler, closing and removing all connected sessions,
// and preventing new sessions from being added.
func (h *StreamableHTTPHandler) Close() error {
	h.mu.Lock()
	defer h.mu.Unlock()
	for _, s := range h.transports {
		s.Close() // assumes #537 implemented. If not same code can be written here.
	}
	h.transports = nil
	return nil
}

Additionally we can have similar Close for SSEHandler

func (s *SSEHandler) Close() error {
	s.mu.Lock()
	defer s.mu.Unlock()
	for _, sess := range s.sessions {
		sess.Close() // assumes #537 implemented. If not same code can be written here.
	}
	s.sessions = nil
	return nil
}

Cc: @jba

IAmSurajBobade avatar Sep 27 '25 16:09 IAmSurajBobade

From proposal meeting: needs investigation to see if there's a more natural way to associate cleanup with the shutdown of the HTTP server.

findleyr avatar Oct 08 '25 15:10 findleyr