Skip to content
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

[BYOM] Handle Invalid Response Format #27345

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jonathansampson
Copy link
Contributor

Resolves brave/brave-browser#43488

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Configure a local server to respond to POST messages with the following:

{"choices":[{"message":{"not_content":"value"}}]}

Configure that endpoint as a BYOM server, and attempt to use it. You should see an error regarding the invalid response format.

image

This commit improves logging around this exception, as well as sets the Error state if/when question-generation fails. This enables Leo to display an error message to the end user.
Introduces a new Leo alert that can be used to inform the user when their configured server has responded with an invalid or otherwise unexpected format.
This commit updates the response handling to no longer assume the presence of a "content" value on the basis of "message" and "choices" being in the response. If a 200 OK response is received, but the structure of the response doesn't match our expectations, we will propagate back to Leo an InvalidResponseFormat error.
@jonathansampson jonathansampson self-assigned this Jan 25, 2025
@jonathansampson jonathansampson requested a review from a team as a code owner January 25, 2025 21:17
@github-actions github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Jan 25, 2025
@jonathansampson jonathansampson changed the title Sampson byom invalid response format [BYOM] Handle Invalid Response Format Jan 25, 2025
@jonathansampson jonathansampson marked this pull request as draft January 25, 2025 21:29
@jonathansampson
Copy link
Contributor Author

Just noticed an issue where the final response from the server could unintentionally trigger the "invalid response format" alert. I'll address that ASAP in another commit.

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CI/storybook-url Deploy storybook and provide a unique URL for each build enhancement feature/leo Leo-related issues and features release-notes/exclude
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BYOM] Handle Invalid Response Formats
2 participants