Skip to content

Commit fda95e1

Browse files
authored
Refactor [Summarizer] Only use invalidResponse for http error responses. (#28441)
fix: forward http error codes
1 parent 8972c78 commit fda95e1

File tree

6 files changed

+37
-7
lines changed

6 files changed

+37
-7
lines changed

BrowserKit/Sources/SummarizeKit/Backend/AppleIntelligence/FoundationModelsSummarizer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ final class FoundationModelsSummarizer: SummarizerProtocol {
6969
/// When `next()` returns nil, the underlying stream has no more data
7070
/// returning nil in turn ends the AsyncThrowingStream
7171
guard let chunk = try await responseStream.next() else { return nil }
72-
guard let stringChunk = chunk as? String else { throw SummarizerError.invalidResponse }
72+
guard let stringChunk = chunk as? String else { throw SummarizerError.invalidChunk }
7373
return stringChunk
7474
} catch {
7575
throw self.mapError(error)

BrowserKit/Sources/SummarizeKit/Backend/LLM/LiteLLMSummarizer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ final class LiteLLMSummarizer: SummarizerProtocol {
7070
case LiteLLMClientError.invalidResponse(let statusCode) where statusCode == 429:
7171
return .rateLimited
7272
case LiteLLMClientError.invalidResponse(let statusCode):
73-
return .unknown(LiteLLMClientError.invalidResponse(statusCode: statusCode))
73+
return .invalidResponse(statusCode: statusCode)
7474
case is CancellationError: return .cancelled
7575
case let e as LiteLLMClientError: return .unknown(e)
7676
default: return .unknown(error)

BrowserKit/Sources/SummarizeKit/Backend/SummarizerError.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ enum SummarizerError: Error, LocalizedError, Sendable {
1212
case busy
1313
case safetyBlocked
1414
case unsupportedLanguage
15-
case invalidResponse
15+
case invalidResponse(statusCode: Int)
16+
case unableToExtractContent
17+
case invalidChunk
1618
case cancelled
1719
case noContent
1820
case unknown(Error)
@@ -30,6 +32,8 @@ enum SummarizerError: Error, LocalizedError, Sendable {
3032
case .cancelled: return ""
3133
case .invalidResponse: return ""
3234
case .noContent: return ""
35+
case .invalidChunk: return ""
36+
case .unableToExtractContent: return ""
3337
case .unknown: return ""
3438
}
3539
}
@@ -40,7 +44,7 @@ enum SummarizerError: Error, LocalizedError, Sendable {
4044
case .contentTooLong: self = .tooLong
4145
case .documentLanguageUnsupported: self = .unsupportedLanguage
4246
case .documentNotReadable: self = .cancelled
43-
case nil: self = .invalidResponse
47+
case nil: self = .unableToExtractContent
4448
}
4549
}
4650
}

BrowserKit/Sources/SummarizeKit/SummarizationError+LocalizedErrorsViewModel.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import Foundation
77
extension SummarizerError {
88
var shouldRetrySummarizing: Bool {
99
return switch self {
10-
case .busy, .noContent, .invalidResponse:
10+
case .busy,
11+
.noContent,
12+
.invalidResponse,
13+
.invalidChunk:
1114
true
1215
default:
1316
false
@@ -20,11 +23,14 @@ extension SummarizerError {
2023
localizedErrors.rateLimitedMessage
2124
case .safetyBlocked:
2225
localizedErrors.unsafeContentMessage
23-
case .unsupportedLanguage, .invalidResponse, .tooLong:
26+
case .unsupportedLanguage,
27+
.invalidResponse,
28+
.tooLong,
29+
.unableToExtractContent:
2430
localizedErrors.summarizationNotAvailableMessage
2531
case .noContent, .busy:
2632
localizedErrors.pageStillLoadingMessage
27-
case .unknown, .cancelled:
33+
case .unknown, .cancelled, .invalidChunk:
2834
localizedErrors.genericErrorMessage
2935
}
3036
}

BrowserKit/Tests/SummarizeKitTests/FoundationModelsSummarizerTests.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ extension SummarizerError: @retroactive Equatable {
125125
(.safetyBlocked, .safetyBlocked),
126126
(.unsupportedLanguage, .unsupportedLanguage),
127127
(.invalidResponse, .invalidResponse),
128+
(.unableToExtractContent, .unableToExtractContent),
129+
(.invalidChunk, .invalidChunk),
130+
(.noContent, .noContent),
128131
(.cancelled, .cancelled):
129132
return true
130133

BrowserKit/Tests/SummarizeKitTests/LiteLLMSummarizerTests.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ final class LiteLLMSummarizerTests: XCTestCase {
2020
}
2121
}
2222

23+
func testSummarizeNonStreamingMapsInvalidResponse() async throws {
24+
let rateLimitError = LiteLLMClientError.invalidResponse(statusCode: 502)
25+
let subject = createSubject(respondWithError: rateLimitError)
26+
await assertSummarizeThrows(.invalidResponse(statusCode: 502)) {
27+
_ = try await subject.summarize("t")
28+
}
29+
}
30+
2331
func testSummarizeNonStreamingMapsUnknownError() async throws {
2432
let randomError = NSError(domain: "Random error", code: 1)
2533
let subject = createSubject(respondWithError: randomError)
@@ -49,6 +57,15 @@ final class LiteLLMSummarizerTests: XCTestCase {
4957
}
5058
}
5159

60+
func testSummarizeStreamedMapsInvalidResponse() async throws {
61+
let rateLimitError = LiteLLMClientError.invalidResponse(statusCode: 567)
62+
let subject = createSubject(respondWithError: rateLimitError)
63+
let stream = subject.summarizeStreamed("t")
64+
await assertSummarizeThrows(.invalidResponse(statusCode: 567)) {
65+
for try await _ in stream { }
66+
}
67+
}
68+
5269
func testSummarizeStreamedMapsUnknownError() async throws {
5370
let randomError = NSError(domain: "Random error", code: 1)
5471
let subject = createSubject(respondWithError: randomError)

0 commit comments

Comments
 (0)