langchaingo icon indicating copy to clipboard operation
langchaingo copied to clipboard

Fix #948 ollamaclient: add scanner error checking in stream function for handling context cancellation

Open nickisworking opened this issue 1 year ago • 3 comments

When using ollamaclient, if the application triggers a context cancel for some reason(i.e. handling user cancellation in case of chatbot), and the cancel signal propagates before making the HTTP call (146 line in ollamaclient.go), an error is returned correctly.

However, if the cancel signal propagates after the HTTP call and response.Body is closed, a nil pointer exception occurs at line 152 of ollamallm.go because the scanner loop is not entered and ollamaclient.ChatResponse is not created.

Therefore, while I considered pre-creating ollamaclient.ChatResponse in the oolamallm package, I decided that propagating the context.Canceled error would be a better approach.

Fixes https://github.com/tmc/langchaingo/issues/948

PR Checklist

  • [x] Read the Contributing documentation.
  • [x] Read the Code of conduct documentation.
  • [x] Name your Pull Request title clearly, concisely, and prefixed with the name of the primarily affected package you changed according to Good commit messages (such as memory: add interfaces for X, Y or util: add whizzbang helpers).
  • [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. Fixes #123).
  • [x] Describes the source of new concepts.
  • [x] References existing implementations as appropriate.
  • [x] Contains test coverage for new functions.
  • [x] Passes all golangci-lint checks.

nickisworking avatar Jul 23 '24 01:07 nickisworking

this would fix this issue #948

nickisworking avatar Jul 23 '24 05:07 nickisworking

@tmc can I get a review? 👀

nickisworking avatar Sep 28 '24 13:09 nickisworking

@tmc it is a simple fix in a situation when context deadline triggers and then cause a segmentation fault on resp.Message.Content.

aria3ppp avatar Oct 25 '24 15:10 aria3ppp