This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Cody Gateway: handle streams with trailing newline in Gemini response#63172
Merged
Conversation
…esponse - Modify `parseGoogleTokenUsage` function to find the last non-empty line in the stream, to handle cases where the stream ends with a newline - Add a test case to cover the scenario where the stream ends with a newline This change modifies the behavior of the `parseGoogleTokenUsage` function to handle streams with trailing newlines, which would fail even if the response is valid because Gemini adds a new line to the end of their stream.
chrsmith
approved these changes
Jun 8, 2024
chrsmith
left a comment
Contributor
There was a problem hiding this comment.
In the interest of getting you unblocked, if you have reproduced the issue and updated the unit tests to now catch the problem. Then let's just get this checked n.
But this parseGoogleTokenUsage function seems a little off.
Can't we just call io.ReadAll and just read it into memory, and then manipulate the data as a string? Needing to deal with scanners is usually tedious and hard to get right. (e.g. lastNonEmptyLine = line.
But I'm sure it would take a non-trivial amount of time to rewrite this to simplify things, so let's just get it fixed.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
CONTEXT: https://sourcegraph.slack.com/archives/C05ABRRGB0B/p1717790701356599
Fix an issue where a valid Gemini response ends with new lines, causing a false alert.
Issue: We are seeing
*errutil.leafError: no Google response foundin Sentry complaining when I run a command or chat in VS Code using google as provider.Cause: Currently the code (added my me) would skip to the last line of the stream response and determine if the response is valid or not, which could be an issue because the stream API could ends the response with an empty new line, where our current logic would fail.
Changes included in this PR:
parseGoogleTokenUsagefunction to find the last non-empty line in the stream, to handle cases where the stream ends with a newlineThis change modifies the behavior of the
parseGoogleTokenUsagefunction to handle streams with trailing newlines, which would fail even if the response is valid because Gemini adds a new line to the end of their stream.Test plan
Make a curl command to the Gemini streaming API to confirm the response ends with a new line:
Copied the response to the test file and confirmed our current test would fail with the same error message:
This is now handled by the newly added test with the actually stream response returned by calling the Gemini API.
Changelog