Skip to content

Fix error with long messages breaking for everyone #89

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

Merged
merged 1 commit into from
May 6, 2025
Merged

Conversation

Alex-Welsh
Copy link
Member

The error went like this:
User submits long message, backend returns BadRequestError, INCLUDE_SYSTEM_PROMPT gets flipped, chat history breaks for everyone.

These changes catch the error properly and tell the user to refresh the page.

@Alex-Welsh Alex-Welsh requested a review from sd109 April 4, 2025 10:15
@Alex-Welsh Alex-Welsh self-assigned this Apr 4, 2025
sd109
sd109 previously requested changes Apr 7, 2025
Copy link
Member

@sd109 sd109 left a comment

Choose a reason for hiding this comment

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

Having looked more closely at the changes I made to the app.py script while upgrading to Gradio 5, I think the correct fix here is actually to replace the else with an elif len(history) > 0 clause here.

Could you give that a try and see if it handles long message errors better without us having to add extra error handling based on the message text (which is fragile since it'll break if vLLM ever change their error message format)

@Alex-Welsh Alex-Welsh marked this pull request as draft April 8, 2025 14:13
@Alex-Welsh
Copy link
Member Author

Untested, will revert draft soon

@Alex-Welsh Alex-Welsh marked this pull request as ready for review April 8, 2025 14:51
@Alex-Welsh
Copy link
Member Author

Confirmed, it fixes the issue

@sd109 sd109 merged commit 78c11de into main May 6, 2025
8 of 13 checks passed
@sd109 sd109 deleted the fix-long-messages branch May 6, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants