Skip to content

Refactor error handling for streaming chat flow#117

Closed
nomisRev wants to merge 2 commits intokpavlov:mainfrom
nomisRev:avoid-dropping-messages
Closed

Refactor error handling for streaming chat flow#117
nomisRev wants to merge 2 commits intokpavlov:mainfrom
nomisRev:avoid-dropping-messages

Conversation

@nomisRev
Copy link
Contributor

Closes #113

  • Removed the Error type from StreamingChatLanguageModelReply and its usage. Currently the error was being emitted twice. Once as an exception, and once as data. We should avoid emitting an event twice, since it only occurs once. It's possible for the user to manually transform the exception into a value using catch { emit(...) } as demonstrated in my other PR. Alternatively we can keep StreamingChatLanguageModelReply.Error but not re-emit again on close(throwable). This is a design decision that needs to be made. I don't have a strong preference to be honest, one or the other is fine. You can go back and forth by using catch or throw.
  • Enforces unbounded channel buffer to avoid dropping messages

Removed the `Error` type from `StreamingChatLanguageModelReply` and its usage. Simplified the chat flow implementation by replacing `callbackFlow` with a `flow` and added buffering with unlimited capacity to improve stream handling.
@kpavlov
Copy link
Owner

kpavlov commented Apr 11, 2025

Tho idea behind emitting error as data was to carry some error details from LLM error response, so it is not an erroneous response per se, but an expected LLM reply.
But the specific exception class with LLM result make sense too, let me look at it one more time

@nomisRev
Copy link
Contributor Author

I don't have a strong preference to be honest, one or the other is fine. You can go back and forth by using catch or throw.

Like I said I think either is fine, in the current scenario with both Error and close(error) it requires the user to handle it twice both the error value, and the actual exception. So I would opt for one or the other, I felt that lifting the "error" into the error path from Flow would make it easier to work with since then you can rely on the existing KotlinX Flow operators to handle any exceptions, or build retry flows, etc. With Error as a value catch, and retry are useless for error handling.

Signed-off-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
@kpavlov
Copy link
Owner

kpavlov commented Jul 20, 2025

I opened another PR #178 to remove the StreamingChatLanguageModelReply.Error due to the conflict

@kpavlov kpavlov closed this Jul 20, 2025
kpavlov added a commit that referenced this pull request Jul 20, 2025
…ption (#178)

Removed the Error type from StreamingChatLanguageModelReply and its
usage. Currently the error was being emitted twice. Once as an
exception, and once as data. We should avoid emitting an event twice,
since it only occurs once. It's possible for the user to manually
transform the exception into a value using catch { emit(...) }

Closes #113

Integrated changes from
#117 by @nomisRev :

> Removed the Error type from StreamingChatLanguageModelReply and its
usage. Currently the error was being emitted twice. Once as an
exception, and once as data. We should avoid emitting an event twice,
since it only occurs once. It's possible for the user to manually
transform the exception into a value using catch { emit(...) } as
demonstrated in [my other
PR](https://github.com/kpavlov/langchain4j-kotlin/pull/116/files#diff-7b90cc7eba5a803d05990cdcd146bd884ffd7a0eb77b2e9d19b13d1e054e3ebfR86).
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.

Flow extension potentially dropping messages

2 participants