diff --git a/CHANGELOG.md b/CHANGELOG.md index 82f6fc33..fbed8f92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Compatibility promise: the public API is additive-only. Anything `public` is dep ### Fixed - `Components.Schemas.ResponseOutputTextAnnotationAddedEvent.description` printed a present `annotation` as `Optional(...)`; it now prints the annotation itself, or `nil`. - `Demo`: an error from a request made while polling an assistant run in Chats was silently ignored and the request repeated every second. It now stops the polling and is shown in the conversation. Link preview failures are logged instead of discarded. +- JSON requests never checked the HTTP status, so an HTML gateway body surfaced as a `DecodingError`, and with `ParsingOptions.relaxed` a status of 400 or above whose body was missing required fields could decode as a successful result. Completion-handler, async, and Combine JSON calls now throw for a status of 400 or above, as raw and streaming calls already did: the decoded `APIErrorResponse` or `GeminiAPIErrorResponse`, or `OpenAIError.statusError` when the body isn't an API error ([#471](https://github.com/MacPaw/OpenAI/pull/471)). ## [0.6.0] - 2026-10-27 diff --git a/Sources/OpenAI/Private/URLResponseHandler.swift b/Sources/OpenAI/Private/URLResponseHandler.swift index 07240f6f..0830f636 100644 --- a/Sources/OpenAI/Private/URLResponseHandler.swift +++ b/Sources/OpenAI/Private/URLResponseHandler.swift @@ -20,30 +20,19 @@ struct URLResponseHandler { } func interceptAndDecode(response: URLResponse, urlRequest: URLRequest, responseData data: Data) throws -> ResultType { - let interceptedData = intercept(response: response, data: data, request: urlRequest) - return try decodeJson(data: interceptedData ?? data) + try decodeJson(data: interceptAndValidate(response: response, urlRequest: urlRequest, error: nil, responseData: data)) } func interceptAndDecode(response: URLResponse?, urlRequest: URLRequest, error: Error?, responseData data: Data?) throws -> ResultType { - let interceptedData = intercept(response: response, data: data, request: urlRequest) - let unwrappedData = try self.unwrap(error: error, originalData: data, interceptedData: interceptedData) - return try decodeJson(data: unwrappedData) + try decodeJson(data: interceptAndValidate(response: response, urlRequest: urlRequest, error: error, responseData: data)) } func interceptAndDecodeRaw(response: URLResponse, urlRequest: URLRequest, responseData data: Data) throws -> Data { - let interceptedData = intercept(response: response, data: data, request: urlRequest) - let finalData = interceptedData ?? data - try throwIfErrorStatus(response: response, data: finalData) - return finalData + try interceptAndValidate(response: response, urlRequest: urlRequest, error: nil, responseData: data) } func interceptAndDecodeRaw(response: URLResponse?, urlRequest: URLRequest, error: Error?, responseData data: Data?) throws -> Data { - let interceptedData = intercept(response: response, data: data, request: urlRequest) - if let error { - throw error - } - try throwIfErrorStatus(response: response, data: interceptedData ?? data) - return try unwrap(error: nil, originalData: data, interceptedData: interceptedData) + try interceptAndValidate(response: response, urlRequest: urlRequest, error: error, responseData: data) } private func decodeJson(data: Data) throws -> ResultType { @@ -51,8 +40,27 @@ struct URLResponseHandler { return try jsonDecoder.decodeResponseData(data) } - /// Raw responses aren't decoded as JSON, so unlike JSON requests they can't fall back to decoding an API error - /// when the expected type fails to decode; check the status code instead, as `StreamingSession` does. + /// Response middleware runs first, then a transport error is thrown unchanged. The status check reads the + /// response and body the middleware returns. An HTTP status of 400 or above is an error for every JSON and raw + /// response: the decoded API or Gemini error, or `OpenAIError.statusError`. The body is returned only after that check, or `OpenAIError.emptyData` when there is none. JSON callers + /// decode that body next, so a 2xx that isn't the result type still falls back to an API or Gemini error. + private func interceptAndValidate(response: URLResponse?, urlRequest: URLRequest, error: Error?, responseData data: Data?) throws -> Data { + let intercepted = intercept(response: response, data: data, request: urlRequest) + if let error { + throw error + } + + let body = intercepted.data ?? data + try throwIfErrorStatus(response: intercepted.response ?? response, data: body) + guard let body else { + throw OpenAIError.emptyData + } + + return body + } + + /// Status >= 400, matching `StreamingSession`: throw a decoded API or Gemini error when the body has one, + /// otherwise `OpenAIError.statusError`. Statuses below 400 are not an error here. private func throwIfErrorStatus(response: URLResponse?, data: Data?) throws { guard let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode >= 400 else { return @@ -64,24 +72,9 @@ struct URLResponseHandler { throw OpenAIError.statusError(response: httpResponse, statusCode: httpResponse.statusCode) } - private func intercept(response: URLResponse?, data: Data?, request: URLRequest) -> Data? { - let (_, interceptedData) = self.middlewares.reduce((response, data)) { current, middleware in + private func intercept(response: URLResponse?, data: Data?, request: URLRequest) -> (response: URLResponse?, data: Data?) { + self.middlewares.reduce((response, data)) { current, middleware in middleware.intercept(response: current.response, request: request, data: current.data) } - - return interceptedData - } - - private func unwrap(error: Error?, originalData: Data?, interceptedData: Data?) throws -> Data { - if let error { - throw error - } - - let finalData = interceptedData ?? originalData - guard let finalData else { - throw OpenAIError.emptyData - } - - return finalData } } diff --git a/Tests/OpenAITests/FilesEndpointTests.swift b/Tests/OpenAITests/FilesEndpointTests.swift index e0666e23..ca11d083 100644 --- a/Tests/OpenAITests/FilesEndpointTests.swift +++ b/Tests/OpenAITests/FilesEndpointTests.swift @@ -305,10 +305,6 @@ extension FilesEndpointTests { """.utf8) } -private final class UncheckedBox: @unchecked Sendable { - var value: Value? -} - extension OpenAIFile { static func mock(id: String = "file-abc123") -> Self { .init( diff --git a/Tests/OpenAITests/JSONResponseStatusTests.swift b/Tests/OpenAITests/JSONResponseStatusTests.swift new file mode 100644 index 00000000..cfed4b16 --- /dev/null +++ b/Tests/OpenAITests/JSONResponseStatusTests.swift @@ -0,0 +1,268 @@ +// +// JSONResponseStatusTests.swift +// OpenAI +// + +import XCTest +#if canImport(FoundationNetworking) +import FoundationNetworking +#endif +@testable import OpenAI + +final class JSONResponseStatusTests: XCTestCase { + + private var openAI: OpenAIProtocol! + private let urlSession = URLSessionMock() + + override func setUp() async throws { + let configuration = OpenAI.Configuration(token: "foo", organizationIdentifier: "bar", timeoutInterval: 14) + self.openAI = OpenAI(configuration: configuration, session: self.urlSession, streamingSessionFactory: MockStreamingSessionFactory()) + } + + func testAsyncChatsThrowsAPIErrorForErrorStatus() async throws { + urlSession.dataTask = .errorResponse(statusCode: 400, data: Self.apiErrorBody) + + do { + _ = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error") + } catch let error as APIErrorResponse { + XCTAssertEqual(error.error.message, "Invalid request") + XCTAssertEqual(error.error.type, "invalid_request_error") + } + } + + func testCompletionChatsThrowsAPIErrorForErrorStatus() throws { + urlSession.dataTask = .errorResponse(statusCode: 400, data: Self.apiErrorBody) + + let expectation = expectation(description: "chats completes") + let result = UncheckedBox>() + openAI.chats(query: chatQuery()) { + result.value = $0 + expectation.fulfill() + } + wait(for: [expectation], timeout: 1) + + XCTAssertThrowsError(try result.value?.get()) { error in + guard let apiError = error as? APIErrorResponse else { + return XCTFail("Expected APIErrorResponse, got \(error)") + } + XCTAssertEqual(apiError.error.message, "Invalid request") + XCTAssertEqual(apiError.error.type, "invalid_request_error") + } + } + + func testAsyncChatsThrowsStatusErrorForHTMLBody() async throws { + urlSession.dataTask = .errorResponse(statusCode: 502, data: Data("Bad Gateway".utf8)) + + do { + _ = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error") + } catch OpenAIError.statusError(_, let statusCode) { + XCTAssertEqual(statusCode, 502) + } + } + + func testCompletionChatsThrowsStatusErrorForHTMLBody() throws { + urlSession.dataTask = .errorResponse(statusCode: 502, data: Data("Bad Gateway".utf8)) + + let expectation = expectation(description: "chats completes") + let result = UncheckedBox>() + openAI.chats(query: chatQuery()) { + result.value = $0 + expectation.fulfill() + } + wait(for: [expectation], timeout: 1) + + XCTAssertThrowsError(try result.value?.get()) { error in + guard case let .statusError(_, statusCode)? = error as? OpenAIError else { + return XCTFail("Expected OpenAIError.statusError, got \(error)") + } + XCTAssertEqual(statusCode, 502) + } + } + + /// `.relaxed` fills the missing `id`, `object`, and `model` strings, so this body decodes as `ChatResult`. + /// A status of 400 must still be an error. `created` and `choices` are not filled by `.relaxed`. + func testRelaxedAsyncChatsThrowsStatusErrorForPartialBody() async throws { + let openAI = makeOpenAI(parsingOptions: .relaxed) + urlSession.dataTask = .errorResponse(statusCode: 400, data: Data(#"{"created":1,"choices":[]}"#.utf8)) + + do { + let result = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error, got \(result)") + } catch OpenAIError.statusError(_, let statusCode) { + XCTAssertEqual(statusCode, 400) + } + } + + /// A status below 400 still decodes. `ModelResult` fills missing `created`, `object`, and `owned_by` only under `.relaxed`. + func testRelaxedAsyncModelDecodesPartialBodyOnSuccessStatus() async throws { + let openAI = makeOpenAI(parsingOptions: .relaxed) + urlSession.dataTask = .successful(with: Data(#"{"id":"gemini-2.0-flash"}"#.utf8)) + + let result = try await openAI.model(query: ModelQuery(model: .gpt3_5Turbo)) + XCTAssertEqual(result, ModelResult(id: "gemini-2.0-flash", created: 0, object: "model", ownedBy: "")) + } + + /// The status guard applies only at 400 and above. A 200 error document still throws through `JSONResponseDecoder`. + func testAsyncChatsThrowsAPIErrorForErrorDocumentOnSuccessStatus() async throws { + urlSession.dataTask = .successful(with: Self.apiErrorBody) + + do { + _ = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error") + } catch let error as APIErrorResponse { + XCTAssertEqual(error.error.message, "Invalid request") + XCTAssertEqual(error.error.type, "invalid_request_error") + } + } + + /// `DataTaskMock.failed` sets a transport error and status 503 with nil data. The transport error is thrown. + func testCompletionChatsThrowsTransportError() throws { + let transportError = URLError(.cannotConnectToHost) + urlSession.dataTask = .failed(with: transportError) + + let expectation = expectation(description: "chats completes") + let result = UncheckedBox>() + openAI.chats(query: chatQuery()) { + result.value = $0 + expectation.fulfill() + } + wait(for: [expectation], timeout: 1) + + XCTAssertThrowsError(try result.value?.get()) { error in + XCTAssertEqual(error as? URLError, transportError) + } + } + + func testAsyncChatsThrowsGeminiAPIErrorForErrorStatus() async throws { + let body = Data(#"[{"error":{"code":400,"message":"Request contains an invalid argument.","status":"INVALID_ARGUMENT"}}]"#.utf8) + urlSession.dataTask = .errorResponse(statusCode: 400, data: body) + + do { + _ = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error") + } catch let error as GeminiAPIErrorResponse { + XCTAssertEqual(error.error.code, 400) + XCTAssertEqual(error.error.message, "Request contains an invalid argument.") + XCTAssertEqual(error.error.status, "INVALID_ARGUMENT") + } + } + + /// A single Gemini error object is not the array `JSONResponseErrorDecoder` accepts, and it is not an + /// `APIErrorResponse` (`type` is required), so a 400 falls through to `OpenAIError.statusError`. + func testAsyncChatsThrowsStatusErrorForSingleGeminiErrorObject() async throws { + let body = Data(#"{"error":{"code":400,"message":"Request contains an invalid argument.","status":"INVALID_ARGUMENT"}}"#.utf8) + urlSession.dataTask = .errorResponse(statusCode: 400, data: body) + + do { + _ = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error") + } catch OpenAIError.statusError(_, let statusCode) { + XCTAssertEqual(statusCode, 400) + } + } + + /// The status check reads the response a middleware returns, not the original one. + func testAsyncChatsThrowsStatusErrorForMiddlewareErrorStatus() async throws { + let openAI = makeOpenAI(middlewares: [StatusOverrideMiddleware(statusCode: 503)]) + urlSession.dataTask = .successful(with: Data("Unavailable".utf8)) + + do { + let result = try await openAI.chats(query: chatQuery()) + XCTFail("Expected an error, got \(result)") + } catch OpenAIError.statusError(_, let statusCode) { + XCTAssertEqual(statusCode, 503) + } + } + + /// A middleware that turns an error status into a success lets the body decode. + func testAsyncModelDecodesWhenMiddlewareReturnsSuccessStatus() async throws { + let openAI = makeOpenAI(middlewares: [StatusOverrideMiddleware(statusCode: 200)]) + let body = Data(#"{"id":"gpt-3.5-turbo","created":1,"object":"model","owned_by":"openai"}"#.utf8) + urlSession.dataTask = .errorResponse(statusCode: 500, data: body) + + let result = try await openAI.model(query: ModelQuery(model: .gpt3_5Turbo)) + XCTAssertEqual(result, ModelResult(id: "gpt-3.5-turbo", created: 1, object: "model", ownedBy: "openai")) + } + +#if canImport(Combine) + @MainActor + func testCombineChatsThrowsAPIErrorForErrorStatus() throws { + let urlSession = URLSessionMockCombine() + let openAI = OpenAI( + configuration: .init(token: "foo", organizationIdentifier: "bar", timeoutInterval: 14), + session: urlSession, + streamingSessionFactory: MockStreamingSessionFactory() + ) + urlSession.dataTask = .errorResponse(statusCode: 400, data: Self.apiErrorBody) + + XCTAssertThrowsError(try awaitPublisher(openAI.chats(query: chatQuery()))) { error in + guard let apiError = error as? APIErrorResponse else { + return XCTFail("Expected APIErrorResponse, got \(error)") + } + XCTAssertEqual(apiError.error.message, "Invalid request") + XCTAssertEqual(apiError.error.type, "invalid_request_error") + } + } + + @MainActor + func testCombineChatsThrowsStatusErrorForHTMLBody() throws { + let urlSession = URLSessionMockCombine() + let openAI = OpenAI( + configuration: .init(token: "foo", organizationIdentifier: "bar", timeoutInterval: 14), + session: urlSession, + streamingSessionFactory: MockStreamingSessionFactory() + ) + urlSession.dataTask = .errorResponse(statusCode: 502, data: Data("Bad Gateway".utf8)) + + XCTAssertThrowsError(try awaitPublisher(openAI.chats(query: chatQuery()))) { error in + guard case let .statusError(_, statusCode)? = error as? OpenAIError else { + return XCTFail("Expected OpenAIError.statusError, got \(error)") + } + XCTAssertEqual(statusCode, 502) + } + } +#endif + + private func makeOpenAI(parsingOptions: ParsingOptions = [], middlewares: [OpenAIMiddleware] = []) -> OpenAI { + let configuration = OpenAI.Configuration( + token: "foo", + organizationIdentifier: "bar", + timeoutInterval: 14, + parsingOptions: parsingOptions + ) + return OpenAI( + configuration: configuration, + session: urlSession, + streamingSessionFactory: MockStreamingSessionFactory(), + middlewares: middlewares + ) + } + + private func chatQuery() -> ChatQuery { + ChatQuery(messages: [.user(.init(content: .string("Hi")))], model: .gpt3_5Turbo) + } + + /// An OpenAI error document: `error.message` and `error.type`. + private static let apiErrorBody = Data(""" + { + "error": { + "message": "Invalid request", + "type": "invalid_request_error", + "param": null, + "code": null + } + } + """.utf8) +} + +/// Replaces the HTTP status of every response it sees. +private struct StatusOverrideMiddleware: OpenAIMiddleware { + let statusCode: Int + + func intercept(response: URLResponse?, request: URLRequest, data: Data?) -> (response: URLResponse?, data: Data?) { + let url = response?.url ?? URL(fileURLWithPath: "") + return (HTTPURLResponse(url: url, statusCode: statusCode, httpVersion: nil, headerFields: nil), data) + } +} diff --git a/Tests/OpenAITests/Mocks/UncheckedBox.swift b/Tests/OpenAITests/Mocks/UncheckedBox.swift new file mode 100644 index 00000000..c4975b5a --- /dev/null +++ b/Tests/OpenAITests/Mocks/UncheckedBox.swift @@ -0,0 +1,9 @@ +// +// UncheckedBox.swift +// OpenAI +// + +/// A mutable box that test callbacks can write a result into. +final class UncheckedBox: @unchecked Sendable { + var value: Value? +}