Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,10 @@ func (c *Client) doJSON(req *http.Request, out any) error {
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
// read up to 1KB of body for error message
errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
if msg := extractAPIErrorMessage(errBody); msg != "" {
return fmt.Errorf("%s: %s", resp.Status, msg)
}
return fmt.Errorf("unexpected status: %s, %s", resp.Status, string(errBody))
}
if out == nil {
Expand All @@ -143,6 +145,31 @@ func (c *Client) doJSON(req *http.Request, out any) error {
return dec.Decode(out)
}

// extractAPIErrorMessage parses a Huma-style JSON error body and returns a
// human-readable string with just the error messages. Returns "" if the body
// cannot be parsed.
func extractAPIErrorMessage(body []byte) string {
var apiErr struct {
Detail string `json:"detail"`
Errors []struct {
Message string `json:"message"`
} `json:"errors"`
}
if json.Unmarshal(body, &apiErr) != nil || (apiErr.Detail == "" && len(apiErr.Errors) == 0) {
return ""
}
var msgs []string
for _, e := range apiErr.Errors {
if e.Message != "" {
msgs = append(msgs, e.Message)
}
}
if len(msgs) > 0 {
return strings.Join(msgs, "; ")
}
return apiErr.Detail
}

func (c *Client) doJsonRequest(method, pathWithQuery string, in, out any) error {
req, err := c.newRequest(method, pathWithQuery)
if err != nil {
Expand Down Expand Up @@ -579,8 +606,8 @@ func asHTTPStatus(err error) int {
return 0
}
errStr := err.Error()
// Parse error format: "unexpected status: 404 Not Found, ..."
// Extract status code from the error message

// Try "unexpected status: CODE ..." (unparsed JSON fallback)
if strings.Contains(errStr, "unexpected status:") {
parts := strings.Split(errStr, "unexpected status: ")
if len(parts) > 1 {
Expand All @@ -590,7 +617,12 @@ func asHTTPStatus(err error) int {
}
}
}
// Also check for "404" or "Not Found" in the error message

// Try "CODE Status Text: message" (parsed API error)
if code, parseErr := strconv.Atoi(strings.Split(errStr, " ")[0]); parseErr == nil {
return code
}

if strings.Contains(errStr, "404") || strings.Contains(errStr, "Not Found") {
return http.StatusNotFound
}
Expand Down
81 changes: 81 additions & 0 deletions internal/client/client_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client

import (
"fmt"
"testing"
)

Expand Down Expand Up @@ -54,6 +55,86 @@ func TestNewClient_BaseURL(t *testing.T) {
}
}

func TestExtractAPIErrorMessage(t *testing.T) {
tests := []struct {
name string
body string
want string
}{
{
"single error message",
`{"title":"Bad Request","status":400,"detail":"Failed to create server","errors":[{"message":"name is required"}]}`,
"name is required",
},
{
"multiple error messages",
`{"title":"Bad Request","status":400,"detail":"Validation failed","errors":[{"message":"name is required"},{"message":"version is invalid"}]}`,
"name is required; version is invalid",
},
{
"falls back to detail when no error messages",
`{"title":"Bad Request","status":400,"detail":"Something went wrong","errors":[]}`,
"Something went wrong",
},
{
"detail only no errors field",
`{"title":"Internal Server Error","status":500,"detail":"Unexpected failure"}`,
"Unexpected failure",
},
{
"skips empty messages",
`{"title":"Bad Request","status":400,"detail":"fail","errors":[{"message":""},{"message":"real error"}]}`,
"real error",
},
{
"invalid JSON returns empty",
`not json at all`,
"",
},
{
"empty body returns empty",
``,
"",
},
{
"no detail or errors returns empty",
`{"title":"Bad Request","status":400}`,
"",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := extractAPIErrorMessage([]byte(tt.body))
if got != tt.want {
t.Errorf("extractAPIErrorMessage() = %q, want %q", got, tt.want)
}
})
}
}

func TestAsHTTPStatus(t *testing.T) {
tests := []struct {
name string
errMsg string
want int
}{
{"parsed API error format", "400 Bad Request: name is required", 400},
{"unparsed fallback format", "unexpected status: 404 Not Found, {\"detail\":\"not found\"}", 404},
{"500 parsed format", "500 Internal Server Error: something broke", 500},
{"contains 404", "something 404 happened", 404},
{"contains Not Found", "resource Not Found", 404},
{"unknown error", "connection refused", 0},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := fmt.Errorf("%s", tt.errMsg)
if got := asHTTPStatus(err); got != tt.want {
t.Errorf("asHTTPStatus(%q) = %d, want %d", tt.errMsg, got, tt.want)
}
})
}
}

func TestNewClient_Token(t *testing.T) {
c := NewClient("http://localhost:12121", "my-secret-token")
if c.token != "my-secret-token" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/agentregistry-dev/agentregistry/internal/cli/common/gitutil"
platformtypes "github.com/agentregistry-dev/agentregistry/internal/registry/platforms/types"
platformutils "github.com/agentregistry-dev/agentregistry/internal/registry/platforms/utils"
"github.com/agentregistry-dev/agentregistry/pkg/models"
v1alpha2 "github.com/kagent-dev/kagent/go/api/v1alpha2"
kmcpv1alpha1 "github.com/kagent-dev/kmcp/api/v1alpha1"
Expand Down Expand Up @@ -471,7 +472,7 @@ func kubernetesTranslateRemoteMCPServer(server *platformtypes.MCPServer) (*v1alp
return nil, fmt.Errorf("remote MCP server config missing for %s", server.Name)
}

url := kubernetesBuildRemoteMCPURL(server.Remote.Host, server.Remote.Port, server.Remote.Path)
url := platformutils.BuildRemoteMCPURL(server.Remote)
return &v1alpha2.RemoteMCPServer{
TypeMeta: metav1.TypeMeta{APIVersion: "kagent.dev/v1alpha2", Kind: "RemoteMCPServer"},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -576,20 +577,6 @@ func kubernetesTranslateAgentConfigMap(agent *platformtypes.Agent) (*corev1.Conf
}, nil
}

func kubernetesBuildRemoteMCPURL(host string, port uint32, path string) string {
host = strings.TrimSpace(host)
if path == "" {
path = "/"
}
if path[0] != '/' {
path = "/" + path
}
if port == 0 {
return fmt.Sprintf("http://%s%s", host, path)
}
return fmt.Sprintf("http://%s:%d%s", host, port, path)
}

func kubernetesAgentConfigMapName(name, version, deploymentID string) string {
base := fmt.Sprintf("%s-agent-config", name)
if version != "" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ func TestKubernetesTranslatePlatformConfig_RemoteMCP(t *testing.T) {
Name: "remote-server",
MCPServerType: platformtypes.MCPServerTypeRemote,
Remote: &platformtypes.RemoteMCPServer{
Host: "example.com",
Port: 8080,
Path: "/mcp",
Scheme: "https",
Host: "example.com",
Port: 8080,
Path: "/mcp",
},
}},
}
Expand All @@ -77,7 +78,7 @@ func TestKubernetesTranslatePlatformConfig_RemoteMCP(t *testing.T) {
if remote.Name != "remote-server" {
t.Errorf("expected name remote-server, got %s", remote.Name)
}
if remote.Spec.URL != "http://example.com:8080/mcp" {
if remote.Spec.URL != "https://example.com:8080/mcp" {
t.Errorf("unexpected URL %s", remote.Spec.URL)
}
}
Expand Down Expand Up @@ -215,9 +216,10 @@ func TestKubernetesTranslatePlatformConfig_NamespaceConsistency(t *testing.T) {
MCPServerType: platformtypes.MCPServerTypeRemote,
Namespace: tt.mcpNamespace,
Remote: &platformtypes.RemoteMCPServer{
Host: "remote-mcp.example.com",
Port: 8080,
Path: "/mcp",
Scheme: "https",
Host: "remote-mcp.example.com",
Port: 8080,
Path: "/mcp",
},
},
{
Expand Down Expand Up @@ -287,9 +289,10 @@ func TestKubernetesTranslatePlatformConfig_DeploymentIDMetadataAndNaming(t *test
MCPServerType: platformtypes.MCPServerTypeRemote,
Namespace: "demo-ns",
Remote: &platformtypes.RemoteMCPServer{
Host: "example.com",
Port: 80,
Path: "/mcp",
Scheme: "http",
Host: "example.com",
Port: 80,
Path: "/mcp",
},
}},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,10 +397,8 @@ func translateLocalAgentGatewayConfig(agentGatewayPort uint16, servers []*platfo

switch server.MCPServerType {
case platformtypes.MCPServerTypeRemote:
mcpTarget.SSE = &platformtypes.SSETargetSpec{
Host: server.Remote.Host,
Port: server.Remote.Port,
Path: server.Remote.Path,
mcpTarget.MCP = &platformtypes.MCPTargetSpec{
Host: platformutils.BuildRemoteMCPURL(server.Remote),
}
case platformtypes.MCPServerTypeLocal:
switch server.Local.TransportType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ func TestUndeploy_RemovesLocalArtifactsWhenRegistryArtifactIsMissing(t *testing.
DeploymentID: deployment.ID,
MCPServerType: platformtypes.MCPServerTypeRemote,
Remote: &platformtypes.RemoteMCPServer{
Host: "example.com",
Port: 443,
Path: "/mcp",
Scheme: "https",
Host: "example.com",
Port: 443,
Path: "/mcp",
},
}

Expand Down
1 change: 1 addition & 0 deletions internal/registry/platforms/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const (
)

type RemoteMCPServer struct {
Scheme string
Host string
Port uint32
Path string
Expand Down
45 changes: 39 additions & 6 deletions internal/registry/platforms/utils/deployment_adapter_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"maps"
"net"
"net/url"
"slices"
"strconv"
Expand Down Expand Up @@ -283,6 +284,7 @@ func translateRemoteMCPServer(
DeploymentID: deploymentID,
MCPServerType: platformtypes.MCPServerTypeRemote,
Remote: &platformtypes.RemoteMCPServer{
Scheme: u.scheme,
Host: u.host,
Port: u.port,
Path: u.path,
Expand Down Expand Up @@ -380,16 +382,20 @@ func translateLocalMCPServer(
}

type parsedURL struct {
host string
port uint32
path string
scheme string
host string
port uint32
path string
}

func parseURL(rawURL string) (*parsedURL, error) {
u, err := url.Parse(rawURL)
if err != nil {
return nil, fmt.Errorf("failed to parse server remote url: %v", err)
}
if u.Scheme != "http" && u.Scheme != "https" {
return nil, fmt.Errorf("unsupported URL scheme %q: only http and https are supported", u.Scheme)
}
portStr := u.Port()
var port uint32
if portStr == "" {
Expand All @@ -407,12 +413,39 @@ func parseURL(rawURL string) (*parsedURL, error) {
}

return &parsedURL{
host: u.Hostname(),
port: port,
path: u.Path,
scheme: u.Scheme,
host: u.Hostname(),
port: port,
path: u.Path,
}, nil
}

// BuildRemoteMCPURL constructs a well-formed URL from a RemoteMCPServer,
// handling IPv6 bracketing and standard-port omission.
func BuildRemoteMCPURL(remote *platformtypes.RemoteMCPServer) string {
scheme := remote.Scheme
if scheme == "" {
scheme = "http"
}
Comment thread
peterj marked this conversation as resolved.

var host string
includePort := (scheme == "https" && remote.Port != 443) || (scheme == "http" && remote.Port != 80)
if includePort {
host = net.JoinHostPort(remote.Host, fmt.Sprintf("%d", remote.Port))
} else if strings.Contains(remote.Host, ":") {
host = "[" + remote.Host + "]"
} else {
host = remote.Host
}

u := &url.URL{
Scheme: scheme,
Host: host,
Path: remote.Path,
}
return u.String()
}

func generateInternalName(server string) string {
name := strings.ToLower(strings.ReplaceAll(server, " ", "-"))
name = strings.ReplaceAll(name, ".", "-")
Expand Down
Loading
Loading