-
Notifications
You must be signed in to change notification settings - Fork 12k
server: fix streaming crashes #13786
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
Conversation
Yeah, it works! No issues so far after some quick testing. The server is no longer crashing. I'll poke around a little more later on when I have some time. |
Will your PR also tackle the issue described in #13774 ? |
@PrideIsLife Taking care of this in #13800, thanks for bringing my attention to it! |
Apologies to anyone tracking this branch, had to amend my email - google.com no more! - and force push! |
Can confirm as well that it fixes my case. |
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.
I can't test right now, but this looks reasonable.
common/chat.cpp
Outdated
// If we didn't extract thoughts, prelude includes them. | ||
builder.add_content(res->prelude); | ||
if (auto res = builder.try_find_regex(end_response_regex)) { | ||
builder.add_content(res->prelude); | ||
// If we didn't extract thoughts, prelude includes them. | ||
} else { |
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.
Since you're not using res
any more and this block is a no-op, perhaps change it to
if (!builder.try_find_regex(end_response_regex)) {
...
}
Co-Authored-By: ochafik <ochafik@google.com>
…3.1 format. still not ideal but hopefully less prone to crash Co-Authored-By: ochafik <ochafik@google.com>
Follow up to #12379
(also see #13785 which fixes non-chat streaming + stop regression)
This (+ other PR) hopefully fixes crashes such as reported here (cc/ @teleprint-me ) and there (cc/ @pwilkin )