Skip to content

Commit b6f5bc9

Browse files
sebalandsebaland
authored
fix(Analytics): Handling certain auth errors as retryable errors (#3322)
1 parent 988eebb commit b6f5bc9

File tree

5 files changed

+195
-13
lines changed

5 files changed

+195
-13
lines changed

‎AmplifyPlugins/Analytics/Sources/AWSPinpointAnalyticsPlugin/Support/Utils/AnalyticsErrorHelper.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ enum AnalyticsErrorHelper {
1414
switch error {
1515
case let error as AnalyticsErrorConvertible:
1616
return error.analyticsError
17+
case let error as AuthError:
18+
return .configuration(error.errorDescription, error.recoverySuggestion, error)
1719
default:
1820
return getDefaultError(error as NSError)
1921
}

‎AmplifyPlugins/Internal/Sources/InternalAWSPinpoint/Analytics/EventRecorder.swift‎

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//
77

88
import Amplify
9+
import AWSCognitoAuthPlugin
910
import AWSPinpoint
1011
import ClientRuntime
1112
import enum AwsCommonRuntimeKit.CommonRunTimeError
@@ -186,25 +187,61 @@ class EventRecorder: AnalyticsEventRecording {
186187
} catch let analyticsError as AnalyticsError {
187188
// This is a known error explicitly thrown inside the do/catch block, so just rethrow it so it can be handled by the consumer
188189
throw analyticsError
190+
} catch let authError as AuthError {
191+
// This means all events were rejected due to an Auth error
192+
log.error("Unable to submit \(pinpointEvents.count) events. Error: \(authError.errorDescription). \(authError.recoverySuggestion)")
193+
switch authError {
194+
case .signedOut,
195+
.sessionExpired:
196+
// Session Expired and Signed Out errors should be retried indefinitely, so we won't update the database
197+
log.verbose("Events will be retried")
198+
case .service:
199+
if case .invalidAccountTypeException = authError.underlyingError as? AWSCognitoAuthError {
200+
// Unsupported Guest Access errors should be retried indefinitely, so we won't update the database
201+
log.verbose("Events will be retried")
202+
} else {
203+
fallthrough
204+
}
205+
default:
206+
if let underlyingError = authError.underlyingError {
207+
// Handle the underlying error accordingly
208+
handleError(underlyingError, for: pinpointEvents)
209+
} else {
210+
// Otherwise just mark all events as dirty
211+
log.verbose("Events will be discarded")
212+
markEventsAsDirty(pinpointEvents)
213+
}
214+
}
215+
216+
// Rethrow the original error so it can be handled by the consumer
217+
throw authError
189218
} catch {
190219
// This means all events were rejected
191-
if isConnectivityError(error) {
192-
// Connectivity errors should be retried indefinitely, so we won't update the database
193-
log.error("Unable to submit \(pinpointEvents.count) events. Error: \(AWSPinpointErrorConstants.deviceOffline.errorDescription)")
194-
} else if isErrorRetryable(error) {
195-
// For retryable errors, increment the events retry count
196-
log.error("Unable to submit \(pinpointEvents.count) events. Error: \(errorDescription(error)).")
197-
incrementRetryCounter(for: pinpointEvents)
198-
} else {
199-
// For remaining errors, mark events as dirty
200-
log.error("Server rejected the submission of \(pinpointEvents.count) events. Error: \(errorDescription(error)).")
201-
markEventsAsDirty(pinpointEvents)
202-
}
220+
log.error("Unable to submit \(pinpointEvents.count) events. Error: \(errorDescription(error)).")
221+
handleError(error, for: pinpointEvents)
203222

204223
// Rethrow the original error so it can be handled by the consumer
205224
throw error
206225
}
207226
}
227+
228+
private func handleError(_ error: Error, for pinpointEvents: [PinpointEvent]) {
229+
if isConnectivityError(error) {
230+
// Connectivity errors should be retried indefinitely, so we won't update the database
231+
log.verbose("Events will be retried")
232+
return
233+
}
234+
235+
if isErrorRetryable(error) {
236+
// For retryable errors, increment the events retry count
237+
log.verbose("Events' retry count will be increased")
238+
incrementRetryCounter(for: pinpointEvents)
239+
} else {
240+
// For remaining errors, mark events as dirty
241+
log.verbose("Events will be discarded")
242+
markEventsAsDirty(pinpointEvents)
243+
}
244+
}
208245

209246
private func isErrorRetryable(_ error: Error) -> Bool {
210247
guard case let modeledError as ModeledError = error else {
@@ -214,6 +251,9 @@ class EventRecorder: AnalyticsEventRecording {
214251
}
215252

216253
private func errorDescription(_ error: Error) -> String {
254+
if isConnectivityError(error) {
255+
return AWSPinpointErrorConstants.deviceOffline.errorDescription
256+
}
217257
switch error {
218258
case let error as ModeledErrorDescribable:
219259
return error.errorDescription

‎AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/EventRecorderTests.swift‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77

88
import XCTest
99
import AWSPinpoint
10+
import AwsCommonRuntimeKit
1011
@testable import Amplify
12+
import ClientRuntime
1113
@_spi(InternalAWSPinpoint) @testable import InternalAWSPinpoint
1214

1315
class EventRecorderTests: XCTestCase {
@@ -26,6 +28,13 @@ class EventRecorderTests: XCTestCase {
2628
XCTFail("Failed to setup EventRecorderTests")
2729
}
2830
}
31+
32+
override func tearDown() {
33+
pinpointClient = nil
34+
endpointClient = nil
35+
storage = nil
36+
recorder = nil
37+
}
2938

3039
/// - Given: a event recorder
3140
/// - When: instance is constructed
@@ -56,4 +65,129 @@ class EventRecorderTests: XCTestCase {
5665
XCTAssertEqual(event, storage.events[0])
5766
XCTAssertEqual(storage.checkDiskSizeCallCount, 2)
5867
}
68+
69+
/// - Given: a event recorder with events saved in the local storage
70+
/// - When: submitAllEvents is invoked and successful
71+
/// - Then: the events are removed from the local storage
72+
func testSubmitAllEvents_withSuccess_shouldRemoveEventsFromStorage() async throws {
73+
Amplify.Logging.logLevel = .verbose
74+
let session = PinpointSession(sessionId: "1", startTime: Date(), stopTime: nil)
75+
storage.events = [
76+
.init(id: "1", eventType: "eventType1", eventDate: Date(), session: session),
77+
.init(id: "2", eventType: "eventType2", eventDate: Date(), session: session)
78+
]
79+
80+
pinpointClient.putEventsResult = .success(.init(eventsResponse: .init(results: [
81+
"endpointId": PinpointClientTypes.ItemResponse(
82+
endpointItemResponse: .init(message: "Accepted", statusCode: 202),
83+
eventsItemResponse: [
84+
"1": .init(message: "Accepted", statusCode: 202),
85+
"2": .init(message: "Accepted", statusCode: 202)
86+
]
87+
)
88+
])))
89+
let events = try await recorder.submitAllEvents()
90+
91+
XCTAssertEqual(events.count, 2)
92+
XCTAssertEqual(pinpointClient.putEventsCount, 1)
93+
XCTAssertTrue(storage.events.isEmpty)
94+
XCTAssertEqual(storage.deleteEventCallCount, 2)
95+
}
96+
97+
/// - Given: a event recorder with events saved in the local storage
98+
/// - When: submitAllEvents is invoked and fails with a non-retryable error
99+
/// - Then: the events are marked as dirty
100+
func testSubmitAllEvents_withRetryableError_shouldSetEventsAsDirty() async throws {
101+
Amplify.Logging.logLevel = .verbose
102+
let session = PinpointSession(sessionId: "1", startTime: Date(), stopTime: nil)
103+
let event1 = PinpointEvent(id: "1", eventType: "eventType1", eventDate: Date(), session: session)
104+
let event2 = PinpointEvent(id: "2", eventType: "eventType2", eventDate: Date(), session: session)
105+
storage.events = [ event1, event2 ]
106+
pinpointClient.putEventsResult = .failure(NonRetryableError())
107+
do {
108+
let events = try await recorder.submitAllEvents()
109+
XCTFail("Expected error")
110+
} catch {
111+
XCTAssertEqual(pinpointClient.putEventsCount, 1)
112+
XCTAssertEqual(storage.events.count, 2)
113+
XCTAssertEqual(storage.deleteEventCallCount, 0)
114+
XCTAssertEqual(storage.eventRetryDictionary.count, 0)
115+
XCTAssertEqual(storage.dirtyEventDictionary.count, 2)
116+
XCTAssertEqual(storage.dirtyEventDictionary["1"], 1)
117+
XCTAssertEqual(storage.dirtyEventDictionary["2"], 1)
118+
}
119+
}
120+
121+
/// - Given: a event recorder with events saved in the local storage
122+
/// - When: submitAllEvents is invoked and fails with a retryable error
123+
/// - Then: the events' retry count is increased
124+
func testSubmitAllEvents_withRetryableError_shouldIncreaseRetryCount() async throws {
125+
Amplify.Logging.logLevel = .verbose
126+
let session = PinpointSession(sessionId: "1", startTime: Date(), stopTime: nil)
127+
let event1 = PinpointEvent(id: "1", eventType: "eventType1", eventDate: Date(), session: session)
128+
let event2 = PinpointEvent(id: "2", eventType: "eventType2", eventDate: Date(), session: session)
129+
storage.events = [ event1, event2 ]
130+
pinpointClient.putEventsResult = .failure(RetryableError())
131+
do {
132+
let events = try await recorder.submitAllEvents()
133+
XCTFail("Expected error")
134+
} catch {
135+
XCTAssertEqual(pinpointClient.putEventsCount, 1)
136+
XCTAssertEqual(storage.events.count, 2)
137+
XCTAssertEqual(storage.deleteEventCallCount, 0)
138+
XCTAssertEqual(storage.eventRetryDictionary.count, 2)
139+
XCTAssertEqual(storage.eventRetryDictionary["1"], 1)
140+
XCTAssertEqual(storage.eventRetryDictionary["2"], 1)
141+
XCTAssertEqual(storage.dirtyEventDictionary.count, 0)
142+
}
143+
}
144+
145+
/// - Given: a event recorder with events saved in the local storage
146+
/// - When: submitAllEvents is invoked and fails with a connectivity error
147+
/// - Then: the events are not removed from the local storage
148+
func testSubmitAllEvents_withConnectivityError_shouldNotIncreaseRetryCount_andNotSetEventsAsDirty() async throws {
149+
Amplify.Logging.logLevel = .verbose
150+
let session = PinpointSession(sessionId: "1", startTime: Date(), stopTime: nil)
151+
let event1 = PinpointEvent(id: "1", eventType: "eventType1", eventDate: Date(), session: session)
152+
let event2 = PinpointEvent(id: "2", eventType: "eventType2", eventDate: Date(), session: session)
153+
storage.events = [ event1, event2 ]
154+
pinpointClient.putEventsResult = .failure(ConnectivityError())
155+
do {
156+
let events = try await recorder.submitAllEvents()
157+
XCTFail("Expected error")
158+
} catch {
159+
XCTAssertEqual(pinpointClient.putEventsCount, 1)
160+
XCTAssertEqual(storage.events.count, 2)
161+
XCTAssertEqual(storage.deleteEventCallCount, 0)
162+
XCTAssertEqual(storage.eventRetryDictionary.count, 0)
163+
XCTAssertEqual(storage.dirtyEventDictionary.count, 0)
164+
}
165+
}
166+
}
167+
168+
private struct RetryableError: Error, ModeledError {
169+
static var typeName = "RetriableError"
170+
static var fault = ErrorFault.client
171+
static var isRetryable = true
172+
static var isThrottling = false
173+
}
174+
175+
private struct NonRetryableError: Error, ModeledError {
176+
static var typeName = "RetriableError"
177+
static var fault = ErrorFault.client
178+
static var isRetryable = false
179+
static var isThrottling = false
180+
}
181+
182+
private class ConnectivityError: NSError {
183+
init() {
184+
super.init(
185+
domain: "ConnectivityError",
186+
code: NSURLErrorNotConnectedToInternet
187+
)
188+
}
189+
190+
required init?(coder: NSCoder) {
191+
super.init(coder: coder)
192+
}
59193
}

‎AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockAnalyticsEventStorage.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
class MockAnalyticsEventStorage: AnalyticsEventStorage {
1111
var deletedEvent: String = ""
12+
var deleteEventCallCount = 0
1213
var deleteDirtyEventCallCount = 0
1314
var initializeStorageCallCount = 0
1415
var deleteOldestEventCallCount = 0
@@ -22,6 +23,8 @@ class MockAnalyticsEventStorage: AnalyticsEventStorage {
2223

2324
func deleteEvent(eventId: String) throws {
2425
deletedEvent = eventId
26+
deleteEventCallCount += 1
27+
events.removeAll { $0.id == eventId }
2528
}
2629

2730
func deleteDirtyEvents() throws {

‎AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockPinpointClient.swift‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,11 @@ class MockPinpointClient: PinpointClientProtocol {
365365
fatalError("Not supported")
366366
}
367367

368+
var putEventsCount = 0
369+
var putEventsResult: Result<PutEventsOutputResponse, Error> = .failure(CancellationError())
368370
func putEvents(input: PutEventsInput) async throws -> PutEventsOutputResponse {
369-
fatalError("Not supported")
371+
putEventsCount += 1
372+
return try putEventsResult.get()
370373
}
371374

372375
func putEventStream(input: PutEventStreamInput) async throws -> PutEventStreamOutputResponse {

0 commit comments

Comments
 (0)