Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(datastore): fix syncQuery for partial success-error responses and update retry mechanism for error codes #3479

Merged
merged 4 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Amplify/Categories/API/Response/GraphQLError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,5 @@ extension GraphQLError {
public let column: Int
}
}

extension GraphQLError: Error { }
31 changes: 31 additions & 0 deletions AmplifyPlugins/Core/AWSPluginsCore/Sync/PaginatedList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,35 @@ public struct PaginatedList<ModelType: Model>: Decodable {
public let items: [MutationSync<ModelType>]
public let nextToken: String?
public let startedAt: Int64?

enum CodingKeys: CodingKey {
case items
case nextToken
case startedAt
}

public init(items: [MutationSync<ModelType>], nextToken: String?, startedAt: Int64?) {
self.items = items
self.nextToken = nextToken
self.startedAt = startedAt
}

public init(from decoder: Decoder) throws {
let values = try decoder.container(keyedBy: CodingKeys.self)
let optimisticDecodedResults = try values.decode([OptimisticDecoded<MutationSync<ModelType>>].self, forKey: .items)
items = optimisticDecodedResults.compactMap { try? $0.result.get() }
nextToken = try values.decode(String?.self, forKey: .nextToken)
startedAt = try values.decode(Int64?.self, forKey: .startedAt)
}
}


fileprivate struct OptimisticDecoded<T: Decodable>: Decodable {
let result: Result<T, Error>

init(from decoder: Decoder) throws {
result = Result(catching: {
try T(from: decoder)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class PaginatedListTests: XCTestCase {
XCTAssertNotNil(paginatedList.startedAt)
XCTAssertNotNil(paginatedList.nextToken)
XCTAssertNotNil(paginatedList.items)
XCTAssertEqual(paginatedList.items.count, 2)
XCTAssert(!paginatedList.items.isEmpty)
XCTAssert(paginatedList.items[0].model.title == "title")
XCTAssert(paginatedList.items[0].syncMetadata.version == 10)
Expand All @@ -94,4 +95,53 @@ class PaginatedListTests: XCTestCase {
}
}

/// - Given: a `Post` Sync query with items, nextToken, and with sync data (startedAt, _version, etc)
/// - When:
/// - some of the JSON items are not able to be decoded to Post
/// - the JSON is decoded into `PaginatedList<Post>`
/// - Then:
/// - the result should contain only valid items of type MutationSync<Post>, startedAt, nextToken.
func testDecodePaginatedListOptimistically() {
let syncQueryJSON = """
{
"items": [
null,
{
"id": "post-id",
"createdAt": "2019-11-27T23:35:39Z",
"_version": 10,
"_lastChangedAt": 1574897753341,
"_deleted": null
},
{
"id": "post-id",
"title": "title",
"content": "post content",
"createdAt": "2019-11-27T23:35:39Z",
"_version": 11,
"_lastChangedAt": 1574897753341,
"_deleted": null
}
],
"startedAt": 1575322600038,
"nextToken": "token"
}
"""
do {
let decoder = JSONDecoder(dateDecodingStrategy: ModelDateFormatting.decodingStrategy)
let data = Data(syncQueryJSON.utf8)
let paginatedList = try decoder.decode(PaginatedList<Post>.self, from: data)
XCTAssertNotNil(paginatedList)
XCTAssertNotNil(paginatedList.startedAt)
XCTAssertNotNil(paginatedList.nextToken)
XCTAssertNotNil(paginatedList.items)
XCTAssertEqual(paginatedList.items.count, 1)
XCTAssert(paginatedList.items[0].model.title == "title")
XCTAssert(paginatedList.items[0].syncMetadata.version == 11)
XCTAssert(paginatedList.items[0].syncMetadata.lastChangedAt == 1_574_897_753_341)
} catch {
XCTFail(error.localizedDescription)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,17 @@ final class InitialSyncOperation: AsynchronousOperation {

let syncQueryResult: SyncQueryResult
switch graphQLResult {
case .success(let queryResult):
syncQueryResult = queryResult

case .failure(.partial(let queryResult, let errors)):
syncQueryResult = queryResult
errors.map { DataStoreError.api(APIError(errorDescription: $0.message, error: $0)) }
.forEach { dataStoreConfiguration.errorHandler($0) }

case .failure(let graphQLResponseError):
finish(result: .failure(DataStoreError.api(graphQLResponseError)))
return
case .success(let queryResult):
syncQueryResult = queryResult
}

let items = syncQueryResult.items
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,18 @@ class SyncMutationToCloudOperation: AsynchronousOperation {
advice = shouldRetryWithDifferentAuthType()
// should retry with a different authType if request failed locally with an AuthError
case .operationError(_, _, let error) where (error as? AuthError) != nil:
advice = shouldRetryWithDifferentAuthType()


// Not all AuthError's are unauthorized errors. If `AuthError.sessionExpired` then
// the request never made it to the server. We should keep trying until the user is signed in.
// Otherwise we may be making the wrong determination to remove this mutation event.
if case .sessionExpired = error as? AuthError {
// Use `userAuthenticationRequired` to ensure advice to retry is true.
advice = requestRetryablePolicy.retryRequestAdvice(urlError: URLError(.userAuthenticationRequired),
httpURLResponse: nil,
attemptNumber: currentAttemptNumber)
} else {
advice = shouldRetryWithDifferentAuthType()
}
case .httpStatusError(_, let httpURLResponse):
advice = requestRetryablePolicy.retryRequestAdvice(urlError: nil,
httpURLResponse: httpURLResponse,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ class RequestRetryablePolicy: RequestRetryable {
.timedOut,
.dataNotAllowed,
.cannotParseResponse,
.networkConnectionLost:
.networkConnectionLost,
.secureConnectionFailed,
.userAuthenticationRequired:
let waitMillis = retryDelayInMillseconds(for: attemptNumber)
return RequestRetryAdvice(shouldRetry: true, retryInterval: .milliseconds(waitMillis))
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ class RequestRetryablePolicyTests: XCTestCase {
assertMilliseconds(retryAdvice.retryInterval, greaterThan: 200, lessThan: 300)
}

func testUserAuthenticationRequiredError() {
let retryableErrorCode = URLError.init(.userAuthenticationRequired)

let retryAdvice = retryPolicy.retryRequestAdvice(urlError: retryableErrorCode,
httpURLResponse: nil,
attemptNumber: 1)

XCTAssert(retryAdvice.shouldRetry)
assertMilliseconds(retryAdvice.retryInterval, greaterThan: 200, lessThan: 300)
}

func testHTTPTooManyRedirectsError() {
let nonRetryableErrorCode = URLError.init(.httpTooManyRedirects)

Expand All @@ -210,6 +221,17 @@ class RequestRetryablePolicyTests: XCTestCase {
XCTAssertEqual(retryAdvice.retryInterval, defaultTimeout)
}

func testSecureConnectionFailedError() {
let retryableErrorCode = URLError.init(.secureConnectionFailed)

let retryAdvice = retryPolicy.retryRequestAdvice(urlError: retryableErrorCode,
httpURLResponse: nil,
attemptNumber: 1)

XCTAssert(retryAdvice.shouldRetry)
assertMilliseconds(retryAdvice.retryInterval, greaterThan: 200, lessThan: 300)
}

func testMaxValueRetryDelay() {
let retryableErrorCode = URLError.init(.timedOut)

Expand Down
Loading