Streaming does not honor http status codes

[SplittyDev]

Describe the bug
When streaming responses (e.g. client.chatsStream(query:)), the stream does not throw when encountering an HTTP error response. That is not only counterintuitive, but also catastrophically wrong.

It breaks the expectation that errors on the server end will cause the stream to throw, makes errors undetectable and client-side error handling impossible, as any for let result in await client.chatsStream(...) loops just silently break, indicating that everything went well.

To Reproduce
Stream a response from a server that returns an http error, for example 500.

Here's a short snippet that exhibits this issue:

do {
  let query: ChatQuery = someValidChatQuery // define a valid `ChatQuery` here
  for try await response in client.chatsStream(query: query) {
    print("Received response")
  }
  print("Finished without error")
} catch {
  print("Error")
}

In the case that the server responds with an HTTP error, the "Error" message will never be printed. Instead, you will just see the "Finished without error" message.

Expected behavior
The AsyncThrowingStream should throw.

Additional context

I have a possible fix, but I'm not sure I like it very much:

diff --git a/Sources/OpenAI/Private/StreamingSession.swift b/Sources/OpenAI/Private/StreamingSession.swift
index fa763f5..379f892 100644
--- a/Sources/OpenAI/Private/StreamingSession.swift
+++ b/Sources/OpenAI/Private/StreamingSession.swift
@@ -46,6 +46,15 @@ final class StreamingSession<ResultType: Codable>: NSObject, Identifiable, URLSe
             onProcessingError?(self, error)
         }
     }
+
+    func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) {
+        if let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode >= 400 {
+            self.onProcessingError?(self, StreamingError.statusError(httpResponse.statusCode))
+            completionHandler(.cancel)
+            return
+        }
+        completionHandler(.allow)
+    }
     
     private func subscribeToParser() {

I've extended StreamingError too:

diff --git a/Sources/OpenAI/Private/StreamingError.swift b/Sources/OpenAI/Private/StreamingError.swift
index 66f77ed..5043fa5 100644
--- a/Sources/OpenAI/Private/StreamingError.swift
+++ b/Sources/OpenAI/Private/StreamingError.swift
@@ -10,4 +10,5 @@ import Foundation
 enum StreamingError: Error {
     case unknownContent
     case emptyContent
+    case statusError(Int)
 }

This works and causes the stream to throw when trying to await the first chunk, but it does not disclose the error properly, because StreamingError is private.

I see the following options:

• Make StreamingError public
• Add a new case to OpenAIError, although it's not really an OpenAI-specific error
• Somehow fit it into APIError? But that seems to make even less sense.

I also want to stress that I'm not sure that this is the best option in general. Maybe there's a much better way to detect and handle HTTP status errors. These findings are the result of a 20-minute investigation into why HTTP status errors fail to throw, and there might be a better way.

[nezhyborets]

@SplittyDev here @eskimo (who's a famous DTS) shows his example on how to detect codes. His implementation looks similar, so I think what you did is good.

As for error... I don't have a guideline on how to approach this. Gut feeling says we shouldn't bloat kinds of errors. The list should be concise, clear and informative. I think I would add a case to OpenAIError. Maybe something like @eskimo showed in his snippet:

enum OpenAIError {
    ...
    case serverSideError(Int)
    ...
}

This case may look too generic and include too little info. But as our requests don't seem to be complex, it should be clear enough what happened, because client specifies the request and clients gets the error for the request.

As for this concern

Add a new case to OpenAIError, although it's not really an OpenAI-specific error

Let's look at it as errors in OpenAIError are not about OpenAI in its original sense, but about our library. So if there'd be not Swift.Error to confuse with, we could just name this enum Error, and it would be OpenAI.Error. Like, any error this lib can throw. (It's not actually the case at the moment because the lib can also throw APIError, but let's put that aside for now)

[nezhyborets]

The only thing I'd mention is if we introduce this change, we should introduce it for regular requests also, so that regular and streaming behave the same in sense of handling status codes

[eskimo]

I'm a different eskimo, sorry :)