diff --git a/core/changelog.md b/core/changelog.md index e9993725b0..6da86c94a1 100644 --- a/core/changelog.md +++ b/core/changelog.md @@ -1 +1,3 @@ -- chore: added case-insensitive helper methods for header and query parameter lookups in HTTPRequest \ No newline at end of file +- chore: added case-insensitive helper methods for header and query parameter lookups in HTTPRequest +- feat: added support for path parameter lookups in HTTPRequest +- fix: missing request type in error response for anthropic SDK integration \ No newline at end of file diff --git a/core/providers/anthropic/errors.go b/core/providers/anthropic/errors.go index 7ce6a96e1b..d7154d5713 100644 --- a/core/providers/anthropic/errors.go +++ b/core/providers/anthropic/errors.go @@ -1,9 +1,9 @@ package anthropic import ( - "encoding/json" "fmt" + "github.com/bytedance/sonic" providerUtils "github.com/maximhq/bifrost/core/providers/utils" schemas "github.com/maximhq/bifrost/core/schemas" "github.com/valyala/fasthttp" @@ -15,15 +15,13 @@ func ToAnthropicChatCompletionError(bifrostErr *schemas.BifrostError) *Anthropic return nil } - // Provide blank strings for nil pointer fields + // Safely extract type and message from nested error errorType := "" - if bifrostErr.Type != nil { - errorType = *bifrostErr.Type - } - - // Safely extract message from nested error message := "" if bifrostErr.Error != nil { + if bifrostErr.Error.Type != nil { + errorType = *bifrostErr.Error.Type + } message = bifrostErr.Error.Message } @@ -48,7 +46,7 @@ func ToAnthropicResponsesStreamError(bifrostErr *schemas.BifrostError) string { anthropicErr := ToAnthropicChatCompletionError(bifrostErr) // Marshal to JSON - jsonData, err := json.Marshal(anthropicErr) + jsonData, err := sonic.Marshal(anthropicErr) if err != nil { return "" } diff --git a/core/providers/gemini/types.go b/core/providers/gemini/types.go index 6d2f85f04e..ac277873c3 100644 --- a/core/providers/gemini/types.go +++ b/core/providers/gemini/types.go @@ -1747,3 +1747,12 @@ type GeminiCountTokensResponse struct { // Output only. List of modalities that were processed in the cached content. CacheTokensDetails []*ModalityTokenCount `json:"cacheTokensDetails,omitempty"` } + +var GeminiRequestSuffixPaths = []string{ + ":streamGenerateContent", + ":generateContent", + ":countTokens", + ":embedContent", + ":batchEmbedContents", + ":predict", +} diff --git a/core/schemas/plugin.go b/core/schemas/plugin.go index 1bd794b870..043fb353af 100644 --- a/core/schemas/plugin.go +++ b/core/schemas/plugin.go @@ -29,11 +29,12 @@ type PluginStatus struct { // Used for plugin HTTP transport interception (supports both native .so and WASM plugins). // This type is pooled for allocation control - use AcquireHTTPRequest and ReleaseHTTPRequest. type HTTPRequest struct { - Method string `json:"method"` - Path string `json:"path"` - Headers map[string]string `json:"headers"` - Query map[string]string `json:"query"` - Body []byte `json:"body"` + Method string `json:"method"` + Path string `json:"path"` + Headers map[string]string `json:"headers"` + Query map[string]string `json:"query"` + Body []byte `json:"body"` + PathParams map[string]string `json:"path_params"` // Path variables extracted from the URL pattern (e.g., {model}) } // CaseInsensitiveHeaderLookup looks up a header key in a case-insensitive manner @@ -46,6 +47,11 @@ func (req *HTTPRequest) CaseInsensitiveQueryLookup(key string) string { return caseInsensitiveLookup(req.Query, key) } +// CaseInsensitivePathParamLookup looks up a path parameter key in a case-insensitive manner +func (req *HTTPRequest) CaseInsensitivePathParamLookup(key string) string { + return caseInsensitiveLookup(req.PathParams, key) +} + // caseInsensitiveLookup looks up a key in a case-insensitive manner for a map of strings // Returns the value if found, otherwise an empty string func caseInsensitiveLookup(data map[string]string, key string) string { @@ -82,8 +88,9 @@ type HTTPResponse struct { var httpRequestPool = sync.Pool{ New: func() any { return &HTTPRequest{ - Headers: make(map[string]string, 16), - Query: make(map[string]string, 8), + Headers: make(map[string]string, 16), + Query: make(map[string]string, 8), + PathParams: make(map[string]string, 4), } }, } @@ -105,6 +112,7 @@ func ReleaseHTTPRequest(req *HTTPRequest) { // Clear the maps clear(req.Headers) clear(req.Query) + clear(req.PathParams) // Reset fields req.Method = "" req.Path = "" diff --git a/plugins/governance/changelog.md b/plugins/governance/changelog.md index 460c23fa1c..281c637ed9 100644 --- a/plugins/governance/changelog.md +++ b/plugins/governance/changelog.md @@ -1 +1,2 @@ -- feat: Fixed weighted provider routing to correctly match provider-prefixed models in allowed lists +- feat: fixed weighted provider routing to correctly match provider-prefixed models in allowed lists +- fix: added support for model lookup in Google GenAI integration by path parameter \ No newline at end of file diff --git a/plugins/governance/main.go b/plugins/governance/main.go index 6b4bbad00d..ddcacb5aeb 100644 --- a/plugins/governance/main.go +++ b/plugins/governance/main.go @@ -12,6 +12,7 @@ import ( "github.com/bytedance/sonic" bifrost "github.com/maximhq/bifrost/core" + "github.com/maximhq/bifrost/core/providers/gemini" "github.com/maximhq/bifrost/core/schemas" "github.com/maximhq/bifrost/framework/configstore" configstoreTables "github.com/maximhq/bifrost/framework/configstore/tables" @@ -288,7 +289,7 @@ func (p *GovernancePlugin) HTTPTransportIntercept(ctx *schemas.BifrostContext, r p.logger.Error("failed to unmarshal request body to check for virtual key: %v", err) return nil, nil } - payload, err = p.loadBalanceProvider(payload, virtualKey) + payload, err = p.loadBalanceProvider(ctx, req, payload, virtualKey) if err != nil { p.logger.Error("failed to load balance provider: %v", err) return nil, nil @@ -304,23 +305,39 @@ func (p *GovernancePlugin) HTTPTransportIntercept(ctx *schemas.BifrostContext, r // loadBalanceProvider loads balances the provider for the request // Parameters: +// - req: The HTTP request // - body: The request body // - virtualKey: The virtual key configuration // // Returns: // - map[string]any: The updated request body // - error: Any error that occurred during processing -func (p *GovernancePlugin) loadBalanceProvider(body map[string]any, virtualKey *configstoreTables.TableVirtualKey) (map[string]any, error) { +func (p *GovernancePlugin) loadBalanceProvider(ctx *schemas.BifrostContext, req *schemas.HTTPRequest, body map[string]any, virtualKey *configstoreTables.TableVirtualKey) (map[string]any, error) { // Check if the request has a model field modelValue, hasModel := body["model"] if !hasModel { - return body, nil + // For genai integration, model is present in URL path instead of the request body + if strings.Contains(req.Path, "/genai") { + modelValue = req.CaseInsensitivePathParamLookup("model") + } else { + return body, nil + } } modelStr, ok := modelValue.(string) if !ok || modelStr == "" { return body, nil } - + var genaiRequestSuffix string + // Remove Google GenAI API endpoint suffixes if present + if strings.Contains(req.Path, "/genai") { + for _, sfx := range gemini.GeminiRequestSuffixPaths { + if before, ok := strings.CutSuffix(modelStr, sfx); ok { + modelStr = before + genaiRequestSuffix = sfx + break + } + } + } // Check if model already has provider prefix (contains "/") if strings.Contains(modelStr, "/") { provider, _ := schemas.ParseModelString(modelStr, "") @@ -393,8 +410,14 @@ func (p *GovernancePlugin) loadBalanceProvider(body map[string]any, virtualKey * if selectedProvider == "" && len(allowedProviderConfigs) > 0 { selectedProvider = schemas.ModelProvider(allowedProviderConfigs[0].Provider) } - // Update the model field in the request body - body["model"] = string(selectedProvider) + "/" + modelStr + // For genai integration, model is present in URL path instead of the request body + if strings.Contains(req.Path, "/genai") { + newModelWithRequestSuffix := string(selectedProvider) + "/" + modelStr + genaiRequestSuffix + ctx.SetValue("model", newModelWithRequestSuffix) + } else { + // Update the model field in the request body + body["model"] = string(selectedProvider) + "/" + modelStr + } // Check if fallbacks field is already present _, hasFallbacks := body["fallbacks"] diff --git a/transports/bifrost-http/handlers/middlewares.go b/transports/bifrost-http/handlers/middlewares.go index 6e4f5062c2..cf25924270 100644 --- a/transports/bifrost-http/handlers/middlewares.go +++ b/transports/bifrost-http/handlers/middlewares.go @@ -110,6 +110,27 @@ func fasthttpToHTTPRequest(ctx *fasthttp.RequestCtx, req *schemas.HTTPRequest) { req.Query[string(key)] = string(value) } + // Copy path parameters from user values + // The fasthttp router stores path variables (like {file_id}, {model}) as user values + // We extract all string user values that are likely path parameters + ctx.VisitUserValuesAll(func(key, value any) { + // Only process string keys and string values + keyStr, keyIsString := key.(string) + valueStr, valueIsString := value.(string) + if !keyIsString || !valueIsString { + return + } + // Skip internal Bifrost system keys and tracing keys + if strings.HasPrefix(keyStr, "bifrost-") || + keyStr == "BifrostContextKeyRequestID" || + keyStr == "trace_id" || + keyStr == "span_id" { + return + } + // Store as path parameter + req.PathParams[keyStr] = valueStr + }) + // Copy body body := ctx.Request.Body() if len(body) > 0 { diff --git a/transports/bifrost-http/handlers/middlewares_test.go b/transports/bifrost-http/handlers/middlewares_test.go index 2feb346f9b..821da7c9e6 100644 --- a/transports/bifrost-http/handlers/middlewares_test.go +++ b/transports/bifrost-http/handlers/middlewares_test.go @@ -827,3 +827,62 @@ func TestFasthttpToHTTPRequest(t *testing.T) { } } } + +// TestFasthttpToHTTPRequest_PathParams tests that path parameters are extracted correctly +func TestFasthttpToHTTPRequest_PathParams(t *testing.T) { + ctx := &fasthttp.RequestCtx{} + + // Set up test data + ctx.Request.Header.SetMethod("GET") + ctx.Request.SetRequestURI("/v1beta/files/file-abc123") + + // Simulate what the fasthttp router does - set path params as user values + ctx.SetUserValue("file_id", "file-abc123") + ctx.SetUserValue("model", "gemini-pro") + + // Set some system values that should be ignored + ctx.SetUserValue("BifrostContextKeyRequestID", "req-123") + ctx.SetUserValue("trace_id", "trace-456") + ctx.SetUserValue("span_id", "span-789") + + // Acquire HTTPRequest from pool + req := schemas.AcquireHTTPRequest() + defer schemas.ReleaseHTTPRequest(req) + + // Call the function + fasthttpToHTTPRequest(ctx, req) + + // Verify path parameters are extracted + expectedPathParams := map[string]string{ + "file_id": "file-abc123", + "model": "gemini-pro", + } + + if len(req.PathParams) != len(expectedPathParams) { + t.Errorf("Expected %d path params, got %d", len(expectedPathParams), len(req.PathParams)) + } + + for key, expectedValue := range expectedPathParams { + if actualValue, exists := req.PathParams[key]; !exists { + t.Errorf("Expected path param '%s' to exist", key) + } else if actualValue != expectedValue { + t.Errorf("Expected path param '%s' to be '%s', got '%s'", key, expectedValue, actualValue) + } + } + + // Verify system keys are NOT in path params + systemKeys := []string{"BifrostContextKeyRequestID", "trace_id", "span_id"} + for _, key := range systemKeys { + if _, exists := req.PathParams[key]; exists { + t.Errorf("System key '%s' should not be in path params", key) + } + } + + // Test the helper method + if fileID := req.CaseInsensitivePathParamLookup("file_id"); fileID != "file-abc123" { + t.Errorf("CaseInsensitivePathParamLookup failed: expected 'file-abc123', got '%s'", fileID) + } + if fileID := req.CaseInsensitivePathParamLookup("FILE_ID"); fileID != "file-abc123" { + t.Errorf("CaseInsensitivePathParamLookup should be case-insensitive: expected 'file-abc123', got '%s'", fileID) + } +} diff --git a/transports/bifrost-http/integrations/genai.go b/transports/bifrost-http/integrations/genai.go index 38d03344e6..1e451b2fa9 100644 --- a/transports/bifrost-http/integrations/genai.go +++ b/transports/bifrost-http/integrations/genai.go @@ -108,7 +108,7 @@ func CreateGenAIRouteConfigs(pathPrefix string) []RouteConfig { } return nil, errors.New("invalid request type") }, - ListModelsResponseConverter: func(ctx *schemas.BifrostContext, resp *schemas.BifrostListModelsResponse) (interface{}, error) { + ListModelsResponseConverter: func(ctx *schemas.BifrostContext, resp *schemas.BifrostListModelsResponse) (interface{}, error) { return gemini.ToGeminiListModelsResponse(resp), nil }, ErrorConverter: func(ctx *schemas.BifrostContext, err *schemas.BifrostError) interface{} { @@ -377,14 +377,7 @@ func extractAndSetModelFromURL(ctx *fasthttp.RequestCtx, bifrostCtx *schemas.Bif isCountTokens := strings.HasSuffix(modelStr, ":countTokens") // Remove Google GenAI API endpoint suffixes if present - for _, sfx := range []string{ - ":streamGenerateContent", - ":generateContent", - ":countTokens", - ":embedContent", - ":batchEmbedContents", - ":predict", - } { + for _, sfx := range gemini.GeminiRequestSuffixPaths { modelStr = strings.TrimSuffix(modelStr, sfx) } diff --git a/transports/bifrost-http/integrations/router.go b/transports/bifrost-http/integrations/router.go index 7d86e22e4c..30d171cb12 100644 --- a/transports/bifrost-http/integrations/router.go +++ b/transports/bifrost-http/integrations/router.go @@ -854,7 +854,6 @@ func (g *GenericRouter) handleBatchRequest(ctx *fasthttp.RequestCtx, config Rout // handleFileRequest handles file API requests (upload, list, retrieve, delete, content) func (g *GenericRouter) handleFileRequest(ctx *fasthttp.RequestCtx, config RouteConfig, req interface{}, fileReq *FileRequest, bifrostCtx *schemas.BifrostContext) { - var response interface{} var err error diff --git a/transports/changelog.md b/transports/changelog.md index e50e8ea93c..c55028b5a0 100644 --- a/transports/changelog.md +++ b/transports/changelog.md @@ -1 +1,3 @@ -- feat: Improved model validation for provider-prefixed model configurations +- feat: improved model validation for provider-prefixed model configurations +- fix: added support for model lookup in Google GenAI integration by path parameter (fixes using VK provider routing for GenAI integration) +- fix: missing request type in error response for anthropic SDK integration \ No newline at end of file