Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions misc/simple-responder/simple-responder.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ func main() {
time.Sleep(wait)
}

bodyBytes, _ := io.ReadAll(c.Request.Body)

Comment on lines +45 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for request body reading.

The error from io.ReadAll is ignored, which could lead to unreliable test behavior if reading the request body fails.

-	bodyBytes, _ := io.ReadAll(c.Request.Body)
+	bodyBytes, err := io.ReadAll(c.Request.Body)
+	if err != nil {
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to read request body"})
+		return
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bodyBytes, _ := io.ReadAll(c.Request.Body)
- bodyBytes, _ := io.ReadAll(c.Request.Body)
+ bodyBytes, err := io.ReadAll(c.Request.Body)
+ if err != nil {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to read request body"})
+ return
+ }
🤖 Prompt for AI Agents
In misc/simple-responder/simple-responder.go around lines 45 to 46, the error
returned by io.ReadAll when reading the request body is ignored. Modify the code
to check the error returned by io.ReadAll and handle it appropriately, such as
returning an error response or logging the error, to ensure reliable behavior
when reading the request body fails.

c.JSON(http.StatusOK, gin.H{
"responseMessage": *responseMessage,
"h_content_length": c.Request.Header.Get("Content-Length"),
"request_body": string(bodyBytes),
})
})

Expand Down
45 changes: 45 additions & 0 deletions proxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"regexp"
"runtime"
"slices"
"sort"
"strconv"
"strings"
Expand All @@ -29,6 +30,9 @@ type ModelConfig struct {

// Limit concurrency of HTTP requests to process
ConcurrencyLimit int `yaml:"concurrencyLimit"`

// Model filters see issue #174
Filters ModelFilters `yaml:"filters"`
}

func (m *ModelConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
Expand Down Expand Up @@ -63,6 +67,46 @@ func (m *ModelConfig) SanitizedCommand() ([]string, error) {
return SanitizeCommand(m.Cmd)
}

// ModelFilters see issue #174
type ModelFilters struct {
StripParams string `yaml:"strip_params"`
}

func (m *ModelFilters) UnmarshalYAML(unmarshal func(interface{}) error) error {
type rawModelFilters ModelFilters
defaults := rawModelFilters{
StripParams: "",
}

if err := unmarshal(&defaults); err != nil {
return err
}

*m = ModelFilters(defaults)
return nil
}

func (f ModelFilters) SanitizedStripParams() ([]string, error) {
if f.StripParams == "" {
return nil, nil
}

params := strings.Split(f.StripParams, ",")
cleaned := make([]string, 0, len(params))

for _, param := range params {
trimmed := strings.TrimSpace(param)
if trimmed == "model" || trimmed == "" {
continue
}
cleaned = append(cleaned, strings.TrimSpace(param))
}

// sort cleaned
slices.Sort(cleaned)
return cleaned, nil
}

type GroupConfig struct {
Swap bool `yaml:"swap"`
Exclusive bool `yaml:"exclusive"`
Expand Down Expand Up @@ -212,6 +256,7 @@ func LoadConfigFromReader(r io.Reader) (Config, error) {
modelConfig.CmdStop = strings.ReplaceAll(modelConfig.CmdStop, macroSlug, macroValue)
modelConfig.Proxy = strings.ReplaceAll(modelConfig.Proxy, macroSlug, macroValue)
modelConfig.CheckEndpoint = strings.ReplaceAll(modelConfig.CheckEndpoint, macroSlug, macroValue)
modelConfig.Filters.StripParams = strings.ReplaceAll(modelConfig.Filters.StripParams, macroSlug, macroValue)
}

// enforce ${PORT} used in both cmd and proxy
Expand Down
3 changes: 3 additions & 0 deletions proxy/config_posix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ models:
assert.Equal(t, "", model1.UseModelName)
assert.Equal(t, 0, model1.ConcurrencyLimit)
}

// default empty filter exists
assert.Equal(t, "", model1.Filters.StripParams)
}

func TestConfig_LoadPosix(t *testing.T) {
Expand Down
25 changes: 25 additions & 0 deletions proxy/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,28 @@ models:
})
}
}

func TestConfig_ModelFilters(t *testing.T) {
content := `
macros:
default_strip: "temperature, top_p"
models:
model1:
cmd: path/to/cmd --port ${PORT}
filters:
strip_params: "model, top_k, ${default_strip}, , ,"
`
config, err := LoadConfigFromReader(strings.NewReader(content))
assert.NoError(t, err)
modelConfig, ok := config.Models["model1"]
if !assert.True(t, ok) {
t.FailNow()
}

// make sure `model` and enmpty strings are not in the list
assert.Equal(t, "model, top_k, temperature, top_p, , ,", modelConfig.Filters.StripParams)
sanitized, err := modelConfig.Filters.SanitizedStripParams()
if assert.NoError(t, err) {
assert.Equal(t, []string{"temperature", "top_k", "top_p"}, sanitized)
}
}
3 changes: 3 additions & 0 deletions proxy/config_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ models:
assert.Equal(t, "", model1.UseModelName)
assert.Equal(t, 0, model1.ConcurrencyLimit)
}

// default empty filter exists
assert.Equal(t, "", model1.Filters.StripParams)
}

func TestConfig_LoadWindows(t *testing.T) {
Expand Down
13 changes: 13 additions & 0 deletions proxy/proxymanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,19 @@ func (pm *ProxyManager) proxyOAIHandler(c *gin.Context) {
}
}

// issue #174 strip parameters from the JSON body
stripParams, err := pm.config.Models[realModelName].Filters.SanitizedStripParams()
if err != nil { // just log it and continue
pm.proxyLogger.Errorf("Error sanitizing strip params string: %s, %s", pm.config.Models[realModelName].Filters.StripParams, err.Error())
} else {
for _, param := range stripParams {
bodyBytes, err = sjson.DeleteBytes(bodyBytes, param)
if err != nil {
pm.sendErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("error deleting parameter %s from request", param))
}
}
}

c.Request.Body = io.NopCloser(bytes.NewBuffer(bodyBytes))

// dechunk it as we already have all the body bytes see issue #11
Expand Down
34 changes: 34 additions & 0 deletions proxy/proxymanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,37 @@ func TestProxyManager_ChatContentLength(t *testing.T) {
assert.Equal(t, "81", response["h_content_length"])
assert.Equal(t, "model1", response["responseMessage"])
}

func TestProxyManager_FiltersStripParams(t *testing.T) {
modelConfig := getTestSimpleResponderConfig("model1")
modelConfig.Filters = ModelFilters{
StripParams: "temperature, model, stream",
}

config := AddDefaultGroupToConfig(Config{
HealthCheckTimeout: 15,
LogLevel: "error",
Models: map[string]ModelConfig{
"model1": modelConfig,
},
})

proxy := New(config)
defer proxy.StopProcesses(StopWaitForInflightRequest)
reqBody := `{"model":"model1", "temperature":0.1, "x_param":"123", "y_param":"abc", "stream":true}`
req := httptest.NewRequest("POST", "/v1/chat/completions", bytes.NewBufferString(reqBody))
w := httptest.NewRecorder()

proxy.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
var response map[string]string
assert.NoError(t, json.Unmarshal(w.Body.Bytes(), &response))

// `temperature` and `stream` are gone but model remains
assert.Equal(t, `{"model":"model1", "x_param":"123", "y_param":"abc"}`, response["request_body"])

// assert.Nil(t, response["temperature"])
// assert.Equal(t, "123", response["x_param"])
// assert.Equal(t, "abc", response["y_param"])
// t.Logf("%v", response)
}
Binary file added ui/misc/logo.acorn
Binary file not shown.