-
Notifications
You must be signed in to change notification settings - Fork 270
#fix llm response is too long,the scanner.Scan can't read #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sunli1223
wants to merge
2
commits into
langgenius:main
Choose a base branch
from
sunli1223:fix_http_request_parse
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
150 changes: 150 additions & 0 deletions
150
internal/utils/http_requests/http_wrapper_reader_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| package http_requests | ||
|
|
||
| import ( | ||
| "io" | ||
| "net/http" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| type mockReader struct { | ||
| chunks []string | ||
| index int | ||
| } | ||
|
|
||
| func (m *mockReader) Read(p []byte) (n int, err error) { | ||
| if m.index >= len(m.chunks) { | ||
| return 0, io.EOF | ||
| } | ||
| n = copy(p, m.chunks[m.index]) | ||
| m.index++ | ||
| if m.index == len(m.chunks) { | ||
| return n, io.EOF | ||
| } | ||
| return n, nil | ||
| } | ||
|
|
||
| func TestParseJsonBody(t *testing.T) { | ||
| t.Run("multiple chunks with newlines", func(t *testing.T) { | ||
| chunks := []string{ | ||
| `{"name": "John",`, | ||
| "\n", | ||
| `"age": 30}`, | ||
| "\n", | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
|
|
||
| assert.Equal(t, "John", result["name"]) | ||
| assert.Equal(t, 30, int(result["age"].(float64))) | ||
| }) | ||
|
|
||
| t.Run("chunks without newlines", func(t *testing.T) { | ||
| chunks := []string{ | ||
| `{"name": "Alice",`, | ||
| `"age": 25}`, | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, "Alice", result["name"]) | ||
| assert.Equal(t, 25, int(result["age"].(float64))) | ||
| }) | ||
|
|
||
| t.Run("chunks with mixed newlines", func(t *testing.T) { | ||
| chunks := []string{ | ||
| `{"name": "Bob",`, | ||
| "\n", | ||
| `"age": 35`, | ||
| `,"city": "New York"}`, | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, "Bob", result["name"]) | ||
| assert.Equal(t, 35, int(result["age"].(float64))) | ||
| assert.Equal(t, "New York", result["city"]) | ||
| }) | ||
|
|
||
| t.Run("last chunk without newline", func(t *testing.T) { | ||
| chunks := []string{ | ||
| `{"name": "Eve",`, | ||
| "\n", | ||
| `"age": 28}`, | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, "Eve", result["name"]) | ||
| assert.Equal(t, 28, int(result["age"].(float64))) | ||
| }) | ||
|
|
||
| t.Run("empty chunks", func(t *testing.T) { | ||
| chunks := []string{ | ||
| "", | ||
| "\n", | ||
| "", | ||
| `{"name": "Charlie"}`, | ||
| "\n", | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, "Charlie", result["name"]) | ||
| }) | ||
|
|
||
| t.Run("invalid JSON", func(t *testing.T) { | ||
| chunks := []string{ | ||
| `{"name": "Invalid`, | ||
| "\n", | ||
| `"age": }`, | ||
| } | ||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result map[string]interface{} | ||
| err := parseJsonBody(resp, &result) | ||
| assert.NotNil(t, err) | ||
| }) | ||
|
|
||
| t.Run("large JSON split across multiple chunks", func(t *testing.T) { | ||
| largeJSON := strings.Repeat(`{"key": "value"},`, 1000) // Create a large JSON array | ||
| largeJSON = "[" + largeJSON[:len(largeJSON)-1] + "]" // Remove last comma and wrap in array brackets | ||
|
|
||
| chunkSize := 100 | ||
| chunks := make([]string, 0, len(largeJSON)/chunkSize+1) | ||
| for i := 0; i < len(largeJSON); i += chunkSize { | ||
| end := i + chunkSize | ||
| if end > len(largeJSON) { | ||
| end = len(largeJSON) | ||
| } | ||
| chunks = append(chunks, largeJSON[i:end]) | ||
| } | ||
|
|
||
| reader := &mockReader{chunks: chunks} | ||
| resp := &http.Response{Body: io.NopCloser(reader)} | ||
|
|
||
| var result []map[string]string | ||
| err := parseJsonBody(resp, &result) | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, 1000, len(result)) | ||
| }) | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, the default buffer max size of bufio.Scanner is
64*1024, it's big enough, I'm not sure what's your specific scenarios, but4*1024should not works.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yeuoly
64k is not enough, as many current LLMs support more than 64k of context. If MCP is used, it will add all the prompt words, the MCP response, and all the bytes of the JSON format structure, making it easy to exceed this limit.
The 4K here is just a buffer to improve performance, and there is no maximum buffer limit.Therefore, the maximum byte size of a line ultimately depends on the context size supported by the LLM and the size of the response returned by the tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, Dify uses stream mode for all the LLM calls, response was split into chunks, each chunk should not have a size larger than 64k.
As for the unlimited buffer, I suppose it's not a good design, it leads to DoS attack. for example, I can create a fake OpenAI server and returns 1G data in a single chunk, that's terrible, it might be a configurable environment variable, but not hardcoded. anyway, the buffer should not to be too large, if you use Dify in personal scenarios, yeah, just make it configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yeuoly
in my case, the llm return a chunk as below,when use tool calling, every chunk include the prompt_messages, the prompt_messages is very big:
{ "data": { "model": "deepseek-v3-250324", "prompt_messages": [ { "role": "system", "content": "system promt(Text in Chinese with a length of 9197 characters.)", "name": "" }, { "role": "user", "content": "user query", "name": "" }, { "role": "assistant", "content": "", "name": "" }, { "role": "tool", "content": "Tool execution result: [{'type': 'text', 'text': 'too response in Chinese with a length of 29008 characters", "name": "hotel_info" } ], "system_fingerprint": "", "delta": { "index": 1, "message": { "role": "assistant", "content": "###", "name": "", "tool_calls": [] }, "usage": null, "finish_reason": null } }, "error": "" }In Go, a Chinese character generally occupies 3 bytes, so although the text doesn't seem too long, multiplying it by 3 could surpass 64K.
@Yeuoly