mcp: export session missing error (#771)
Fix: #715 - export session missing error - modify `errSessionMissing` to `ErrSessionMissing` - add unit test about session missing error
This commit is contained in:
parent
df4f0d56a4
commit
7ecd8049c6
4 changed files with 55 additions and 18 deletions
|
|
@ -1540,17 +1540,6 @@ type streamableClientConn struct {
|
|||
sessionID string
|
||||
}
|
||||
|
||||
// errSessionMissing distinguishes if the session is known to not be present on
|
||||
// the server (see [streamableClientConn.fail]).
|
||||
//
|
||||
// TODO(rfindley): should we expose this error value (and its corresponding
|
||||
// API) to the user?
|
||||
//
|
||||
// The spec says that if the server returns 404, clients should reestablish
|
||||
// a session. For now, we delegate that to the user, but do they need a way to
|
||||
// differentiate a 'NotFound' error from other errors?
|
||||
var errSessionMissing = errors.New("session not found")
|
||||
|
||||
var _ clientConnection = (*streamableClientConn)(nil)
|
||||
|
||||
func (c *streamableClientConn) sessionUpdated(state clientSessionState) {
|
||||
|
|
@ -1629,7 +1618,7 @@ func (c *streamableClientConn) connectStandaloneSSE() {
|
|||
// If err is non-nil, it is terminal, and subsequent (or pending) Reads will
|
||||
// fail.
|
||||
//
|
||||
// If err wraps errSessionMissing, the failure indicates that the session is no
|
||||
// If err wraps ErrSessionMissing, the failure indicates that the session is no
|
||||
// longer present on the server, and no final DELETE will be performed when
|
||||
// closing the connection.
|
||||
func (c *streamableClientConn) fail(err error) {
|
||||
|
|
@ -1896,9 +1885,9 @@ func (c *streamableClientConn) checkResponse(requestSummary string, resp *http.R
|
|||
// which it MUST respond to requests containing that session ID with HTTP
|
||||
// 404 Not Found."
|
||||
if resp.StatusCode == http.StatusNotFound {
|
||||
// Return an errSessionMissing to avoid sending a redundant DELETE when the
|
||||
// Return an ErrSessionMissing to avoid sending a redundant DELETE when the
|
||||
// session is already gone.
|
||||
return fmt.Errorf("%s: failed to connect (session ID: %v): %w", requestSummary, c.sessionID, errSessionMissing)
|
||||
return fmt.Errorf("%s: failed to connect (session ID: %v): %w", requestSummary, c.sessionID, ErrSessionMissing)
|
||||
}
|
||||
// Transient server errors (502, 503, 504, 429) should not break the connection.
|
||||
// Wrap them with ErrRejected so the jsonrpc2 layer doesn't set writeErr.
|
||||
|
|
@ -2055,7 +2044,7 @@ func (c *streamableClientConn) connectSSE(ctx context.Context, lastEventID strin
|
|||
// Close implements the [Connection] interface.
|
||||
func (c *streamableClientConn) Close() error {
|
||||
c.closeOnce.Do(func() {
|
||||
if errors.Is(c.failure(), errSessionMissing) {
|
||||
if errors.Is(c.failure(), ErrSessionMissing) {
|
||||
// If the session is missing, no need to delete it.
|
||||
} else {
|
||||
req, err := http.NewRequestWithContext(c.ctx, http.MethodDelete, c.url, nil)
|
||||
|
|
|
|||
|
|
@ -161,7 +161,7 @@ The client must handle two response formats from POST requests:
|
|||
|
||||
- DELETE: Terminate the session
|
||||
- Used by [streamableClientConn.Close]
|
||||
- Skipped if session is already known to be gone ([errSessionMissing])
|
||||
- Skipped if session is already known to be gone ([ErrSessionMissing])
|
||||
|
||||
# Error Handling
|
||||
|
||||
|
|
@ -173,7 +173,7 @@ Errors are categorized and handled differently:
|
|||
- Triggers reconnection in [streamableClientConn.handleSSE]
|
||||
|
||||
2. Terminal (breaks the connection):
|
||||
- 404 Not Found: Session terminated by server ([errSessionMissing])
|
||||
- 404 Not Found: Session terminated by server ([ErrSessionMissing])
|
||||
- Message decode errors: Protocol violation
|
||||
- Context cancellation: Client closed connection
|
||||
- Mismatched session IDs: Protocol error
|
||||
|
|
@ -183,7 +183,7 @@ Terminal errors are stored via [streamableClientConn.fail] and returned by
|
|||
subsequent [streamableClientConn.Read] calls. The [streamableClientConn.failed]
|
||||
channel signals that the connection is broken.
|
||||
|
||||
Special case: [errSessionMissing] indicates the server has terminated the session,
|
||||
Special case: [ErrSessionMissing] indicates the server has terminated the session,
|
||||
so [streamableClientConn.Close] skips the DELETE request.
|
||||
|
||||
# Protocol Version Header
|
||||
|
|
|
|||
|
|
@ -2280,3 +2280,47 @@ data: {"jsonrpc":"2.0","method":"test2","params":{}}
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
func Test_ExportErrSessionMissing(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
// 1. Setup server
|
||||
impl := &Implementation{Name: "test", Version: "1.0.0"}
|
||||
server := NewServer(impl, nil)
|
||||
handler := NewStreamableHTTPHandler(func(r *http.Request) *Server { return server }, nil)
|
||||
ts := httptest.NewServer(handler)
|
||||
defer ts.Close()
|
||||
|
||||
// 2. Setup client
|
||||
clientTransport := &StreamableClientTransport{
|
||||
Endpoint: ts.URL,
|
||||
}
|
||||
client := NewClient(impl, nil)
|
||||
session, err := client.Connect(ctx, clientTransport, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Connect failed: %v", err)
|
||||
}
|
||||
defer session.Close()
|
||||
|
||||
// 3. Manually invalidate session on server
|
||||
handler.mu.Lock()
|
||||
if len(handler.sessions) != 1 {
|
||||
handler.mu.Unlock()
|
||||
t.Fatalf("expected 1 session, got %d", len(handler.sessions))
|
||||
}
|
||||
for id := range handler.sessions {
|
||||
delete(handler.sessions, id)
|
||||
}
|
||||
handler.mu.Unlock()
|
||||
|
||||
// 4. Try to call a tool (or any request)
|
||||
_, err = session.ListTools(ctx, nil)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
|
||||
// 5. Verify it's ErrSessionMissing
|
||||
if !errors.Is(err, ErrSessionMissing) {
|
||||
t.Errorf("expected error to wrap ErrSessionMissing, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,6 +24,10 @@ import (
|
|||
// is closed or in the process of closing.
|
||||
var ErrConnectionClosed = errors.New("connection closed")
|
||||
|
||||
// ErrSessionMissing is returned when the session is known to not be present on
|
||||
// the server.
|
||||
var ErrSessionMissing = errors.New("session not found")
|
||||
|
||||
// A Transport is used to create a bidirectional connection between MCP client
|
||||
// and server.
|
||||
//
|
||||
|
|
|
|||
Loading…
Reference in a new issue