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

fix(stream): Update "IterableReadableStream" pull method to enqueue nullish data (e.g. empty string) #2839

Merged

Conversation

pyokang
Copy link
Contributor

@pyokang pyokang commented Oct 9, 2023

This PR is to address the issue our team identified while we were integrating BaseLLM with our own AI to enable the streaming capability. The issue we faced was, after we implemented *_streamResponseChunks generator function, we noticed it did not terminated in many cases.

After investigation, we found out that our response stream would end by yielding empty string, and this behavior, paired with IterableReadableStream's pull method, which only enqueue's value if it is not nullish, will hang the generator. Hence preventing it from terminating successfully.

…ullish data (e.g. empty string)
@vercel
Copy link

vercel bot commented Oct 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
langchainjs-docs ✅ Ready (Inspect) Visit Preview Oct 9, 2023 5:50pm

@dosubot dosubot bot added the auto:bug Related to a bug, vulnerability, unexpected error with an existing feature label Oct 9, 2023
@jacoblee93
Copy link
Collaborator

jacoblee93 commented Oct 9, 2023

Phenomenal catch - I think this makes more sense to check for !== undefined though? I'll update.

I think what you have is the correct behavior

@jacoblee93 jacoblee93 self-assigned this Oct 9, 2023
@jacoblee93 jacoblee93 added the lgtm PRs that are ready to be merged as-is label Oct 9, 2023
@jacoblee93 jacoblee93 merged commit 4d73366 into langchain-ai:main Oct 9, 2023
@jacoblee93
Copy link
Collaborator

Huge fix, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:bug Related to a bug, vulnerability, unexpected error with an existing feature lgtm PRs that are ready to be merged as-is
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants