zmedelis / bosquet

Tooling to build LLM applications: prompt templating and composition, agents, LLM memory, and other instruments for builders of AI applications.
https://zmedelis.github.io/bosquet/
Eclipse Public License 1.0
280 stars 19 forks source link

(certain) Exceptions from openai LLMs are handled wrongly and result in other exception (original is hidden) #32

Closed behrica closed 1 year ago

behrica commented 1 year ago

I hot-fixed a bit the code related to #30, and came to a point where it indeed tried a call to Azure, which (correctly) gave an exception :

#error {
 :cause nil
 :via
 [{:type clojure.lang.ExceptionInfo
   :message Interceptor Exception: 
   :data {:execution-id #uuid "bee7c345-b673-4b5d-86b9-f9a54ff3eb59", :stage :leave, :interceptor :wkok.openai-clojure.sse/perform-sse-capable-request, :type java.net.ConnectException, :exception #error {
 :cause nil
 :via
 [{:type java.net.ConnectException
   :message nil
   :at [jdk.internal.net.http.HttpClientImpl send HttpClientImpl.java 573]}
  {:type java.net.ConnectException
   :message nil
   :at [jdk.internal.net.http.common.Utils toConnectException Utils.java 1047]}
  {:type java.nio.channels.UnresolvedAddressException
   :message nil
   :at [sun.nio.ch.Net checkAddress Net.java 149]}]
 :trace
 [[sun.nio.ch.Net checkAddress Net.java 149]
  [sun.nio.ch.Net checkAddress Net.java 157]
  [sun.nio.ch.SocketChannelImpl checkRemote SocketChannelImpl.java 816]
  [sun.nio.ch.SocketChannelImpl connect SocketChannelImpl.java 839]
  [jdk.internal.net.http.PlainHttpConnection lambda$connectAsync$0 PlainHttpConnection.java 183]
  [java.security.AccessController doPrivileged AccessController.java 569]
  [jdk.internal.net.http.PlainHttpConnection connectAsync PlainHttpConnection.java 185]
  [jdk.internal.net.http.Http1E

This was wrongly handled here: https://github.com/zmedelis/bosquet/blob/95d410ae5db8e0b5d544b9f79e6cafabf9f01b7a/src/bosquet/llm/openai.clj#L79

therefore "hidden" and resulted in this exception:

#error {
 :cause "Additional data must be non-nil."
 :via
 [{:type java.lang.IllegalArgumentException
   :message "Additional data must be non-nil."
   :at [clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 31]}]
 :trace
 [[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 31]
  [clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 22]
  [clojure.core$ex_info invokeStatic "core.clj" 4807]
  [clojure.core$ex_info invoke "core.clj" 4807]
  [bosquet.llm.openai$complete invokeStatic "openai.clj" 82]
  [bosquet.llm.openai$complete invoke "openai.clj" 42]
  [bosquet.llm.openai.OpenAI generate "openai.clj" 98]

So something goes wrong in the exception handling above. (L79)

behrica commented 1 year ago

In case of Azure, the exception has nil for (-> e ex-data) so, the construction of new ex-info fails.

zmedelis commented 1 year ago

Thanks for debugging this.

See my commit, it should at least not crash on Azure error, but if you could provide the PR which extracts whatever is needed for ex-info extra exception data map so that when re-thrown it is maximally informative.

behrica commented 1 year ago

We will in this point have exceptions from all type of LLMs. So we should make minimal assumptions what to get.

behrica commented 1 year ago

Looking at 7e4b23239e0b0e453c22e2d443aa4df6845a1523 I would have done the same, Assuming 2 cases, one with existing ex-data , one without.

To assume "more" then having a exception message might break other LLMs.

So I would consider this done.

behrica commented 1 year ago

see #37 It seems better then your attempt, as it keeps exception root case (java.net.Exception)