Retry most network failures in RegistrationCoordinator
* Retry most network failures in RegistrationCoordinator * Update the way we identify network errors to be more flexible * Retry remaining requests * Automatically retry registration requests if time intervals are short * fix error types not compiling on kbs * Add test for reg recovery password reg with retries * fix extra t
This commit is contained in:
parent
a6b2326ef3
commit
cd63f67b3e
@ -191,6 +191,7 @@ extension Registration {
|
||||
public enum SyncPushTokensResult {
|
||||
case success
|
||||
case pushUnsupported(description: String)
|
||||
case networkError
|
||||
case genericError
|
||||
}
|
||||
}
|
||||
@ -238,6 +239,9 @@ public class _RegistrationCoordinator_PushRegistrationManagerWrapper: _Registrat
|
||||
return job.run()
|
||||
.map(on: SyncScheduler()) { return .success }
|
||||
.recover(on: SyncScheduler()) { error -> Guarantee<Registration.SyncPushTokensResult> in
|
||||
if error.isNetworkConnectivityFailure {
|
||||
return .value(.networkError)
|
||||
}
|
||||
switch error {
|
||||
case PushRegistrationError.pushNotSupported(let description):
|
||||
return .value(.pushUnsupported(description: description))
|
||||
|
||||
@ -258,10 +258,22 @@ extension RegistrationCoordinatorImpl {
|
||||
public static func makeEnableReglockRequest(
|
||||
reglockToken: String,
|
||||
signalService: OWSSignalServiceProtocol,
|
||||
schedulers: Schedulers
|
||||
schedulers: Schedulers,
|
||||
retriesLeft: Int = RegistrationCoordinatorImpl.Constants.networkErrorRetries
|
||||
) -> Promise<Void> {
|
||||
let request = OWSRequestFactory.enableRegistrationLockV2Request(token: reglockToken)
|
||||
return signalService.urlSessionForMainSignalService().promiseForTSRequest(request).asVoid()
|
||||
.recover(on: schedulers.sync) { error in
|
||||
if error.isNetworkConnectivityFailure, retriesLeft > 0 {
|
||||
return makeEnableReglockRequest(
|
||||
reglockToken: reglockToken,
|
||||
signalService: signalService,
|
||||
schedulers: schedulers,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .init(error: error)
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns nil error if success.
|
||||
@ -270,7 +282,8 @@ extension RegistrationCoordinatorImpl {
|
||||
authUsername: String,
|
||||
authPassword: String,
|
||||
signalService: OWSSignalServiceProtocol,
|
||||
schedulers: Schedulers
|
||||
schedulers: Schedulers,
|
||||
retriesLeft: Int = RegistrationCoordinatorImpl.Constants.networkErrorRetries
|
||||
) -> Guarantee<Error?> {
|
||||
let request = RegistrationRequestFactory.updatePrimaryDeviceAccountAttributesRequest(
|
||||
attributes,
|
||||
@ -286,6 +299,16 @@ extension RegistrationCoordinatorImpl {
|
||||
return nil
|
||||
}
|
||||
.recover(on: schedulers.sync) { error in
|
||||
if error.isNetworkConnectivityFailure, retriesLeft > 0 {
|
||||
return makeUpdateAccountAttributesRequest(
|
||||
attributes,
|
||||
authUsername: authUsername,
|
||||
authPassword: authPassword,
|
||||
signalService: signalService,
|
||||
schedulers: schedulers,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(error)
|
||||
}
|
||||
}
|
||||
@ -307,12 +330,12 @@ extension RegistrationCoordinatorImpl {
|
||||
)
|
||||
}
|
||||
.recover(on: schedulers.sharedBackground) { (error: Error) -> Guarantee<ResponseType> in
|
||||
if error.isNetworkConnectivityFailure {
|
||||
return .value(networkFailureError)
|
||||
}
|
||||
guard let error = error as? OWSHTTPError else {
|
||||
return .value(fallbackError)
|
||||
}
|
||||
if case .networkFailure = error {
|
||||
return .value(networkFailureError)
|
||||
}
|
||||
let response = handler(
|
||||
error.responseStatusCode,
|
||||
error.responseHeaders?.value(forHeader: Constants.retryAfterHeader),
|
||||
|
||||
@ -512,6 +512,13 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
}
|
||||
|
||||
// Update the account attributes once, now, at the end.
|
||||
return updateAccountAttributesAndFinish(accountIdentity: accountIdentity)
|
||||
}
|
||||
|
||||
private func updateAccountAttributesAndFinish(
|
||||
accountIdentity: AccountIdentity,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return Service
|
||||
.makeUpdateAccountAttributesRequest(
|
||||
makeAccountAttributes(authToken: accountIdentity.authToken),
|
||||
@ -534,7 +541,13 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
self.storageServiceManager.backupPendingChanges()
|
||||
return .value(.done)
|
||||
}
|
||||
// TODO[Registration]: handle account attributes update error?
|
||||
if error.isNetworkConnectivityFailure, retriesLeft > 0 {
|
||||
return self.updateAccountAttributesAndFinish(
|
||||
accountIdentity: accountIdentity,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
// TODO[Registration]: what should we do with a non-transiest account attributes update error?
|
||||
Logger.error("Failed to register due to failed account attributes update: \(error)")
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
@ -689,7 +702,11 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
)
|
||||
}
|
||||
|
||||
private func registerForRegRecoveryPwPath(regRecoveryPw: String, e164: String) -> Guarantee<RegistrationStep> {
|
||||
private func registerForRegRecoveryPwPath(
|
||||
regRecoveryPw: String,
|
||||
e164: String,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
if inMemoryState.pinFromUser == nil {
|
||||
// We need the user to confirm their pin.
|
||||
// TODO[Registration]: set state that tells the pin entry controller
|
||||
@ -703,14 +720,18 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
).then(on: schedulers.main) { [weak self] accountResponse in
|
||||
return self?.handleCreateAccountResponseFromRegRecoveryPassword(
|
||||
accountResponse,
|
||||
e164: e164
|
||||
regRecoveryPw: regRecoveryPw,
|
||||
e164: e164,
|
||||
retriesLeft: retriesLeft
|
||||
) ?? .value(.showGenericError)
|
||||
}
|
||||
}
|
||||
|
||||
private func handleCreateAccountResponseFromRegRecoveryPassword(
|
||||
_ response: AccountResponse,
|
||||
e164: String
|
||||
regRecoveryPw: String,
|
||||
e164: String,
|
||||
retriesLeft: Int
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
// TODO[Registration] handle error case for rejected e164.
|
||||
switch response {
|
||||
@ -764,8 +785,20 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
// Now we have to start a session; its the only way to recover.
|
||||
return self.startSession(e164: e164)
|
||||
|
||||
case .retryAfter:
|
||||
// TODO[Registration] handle retries, possibly automatically if short enough.
|
||||
case .retryAfter(let timeInterval):
|
||||
if timeInterval < Constants.autoRetryInterval {
|
||||
return Guarantee
|
||||
.after(on: self.schedulers.sharedBackground, seconds: timeInterval)
|
||||
.then(on: self.schedulers.sync) { [weak self] in
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.registerForRegRecoveryPwPath(
|
||||
regRecoveryPw: regRecoveryPw,
|
||||
e164: e164
|
||||
)
|
||||
}
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
|
||||
case .deviceTransferPossible:
|
||||
@ -774,7 +807,13 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return nextStep()
|
||||
|
||||
case .networkError:
|
||||
// TODO[Registration]: handle retries
|
||||
if retriesLeft > 0 {
|
||||
return registerForRegRecoveryPwPath(
|
||||
regRecoveryPw: regRecoveryPw,
|
||||
e164: e164,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
|
||||
case .genericError:
|
||||
@ -824,7 +863,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
|
||||
private func restoreKBSMasterSecretForAuthCredentialPath(
|
||||
pin: String,
|
||||
credential: KBSAuthCredential
|
||||
credential: KBSAuthCredential,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
kbs.restoreKeysAndBackup(pin: pin, authMethod: .kbsAuth(credential, backup: nil))
|
||||
.then(on: schedulers.main) { [weak self] result -> Guarantee<RegistrationStep> in
|
||||
@ -847,6 +887,15 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
// recover. Give it all up and wipe our KBS info.
|
||||
self.wipeInMemoryStateToPreventKBSPathAttempts()
|
||||
return self.nextStep()
|
||||
case .networkError:
|
||||
if retriesLeft > 0 {
|
||||
return self.restoreKBSMasterSecretForAuthCredentialPath(
|
||||
pin: pin,
|
||||
credential: credential,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
@ -873,6 +922,17 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return .value(.phoneNumberEntry)
|
||||
}
|
||||
// Check the candidates.
|
||||
return makeKBSAuthCredentialCheckRequest(
|
||||
kbsAuthCredentialCandidates: kbsAuthCredentialCandidates,
|
||||
e164: e164
|
||||
)
|
||||
}
|
||||
|
||||
private func makeKBSAuthCredentialCheckRequest(
|
||||
kbsAuthCredentialCandidates: [KBSAuthCredential],
|
||||
e164: String,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return Service.makeKBSAuthCheckRequest(
|
||||
e164: e164,
|
||||
candidateCredentials: kbsAuthCredentialCandidates,
|
||||
@ -885,7 +945,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return self.handleKBSAuthCredentialCheckResponse(
|
||||
response,
|
||||
kbsAuthCredentialCandidates: kbsAuthCredentialCandidates,
|
||||
e164: e164
|
||||
e164: e164,
|
||||
retriesLeft: retriesLeft
|
||||
)
|
||||
}
|
||||
}
|
||||
@ -893,13 +954,20 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
private func handleKBSAuthCredentialCheckResponse(
|
||||
_ response: Service.KBSAuthCheckResponse,
|
||||
kbsAuthCredentialCandidates: [KBSAuthCredential],
|
||||
e164: String
|
||||
e164: String,
|
||||
retriesLeft: Int
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
var matchedCredential: KBSAuthCredential?
|
||||
var credentialsToDelete = [KBSAuthCredential]()
|
||||
switch response {
|
||||
case .networkError:
|
||||
// TODO[Registration]: handle retries
|
||||
if retriesLeft > 0 {
|
||||
return makeKBSAuthCredentialCheckRequest(
|
||||
kbsAuthCredentialCandidates: kbsAuthCredentialCandidates,
|
||||
e164: e164,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
self.inMemoryState.kbsAuthCredentialCandidates = nil
|
||||
return self.nextStep()
|
||||
case .genericError:
|
||||
@ -937,15 +1005,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
private func nextStepForSessionPath(_ session: RegistrationSession) -> Guarantee<RegistrationStep> {
|
||||
if session.verified {
|
||||
// We have to complete registration.
|
||||
return self.makeRegisterOrChangeNumberRequest(
|
||||
.sessionId(session.id),
|
||||
e164: session.e164
|
||||
).then(on: schedulers.main) { [weak self] response in
|
||||
guard let self = self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.handleCreateAccountResponseFromSession(response)
|
||||
}
|
||||
return self.makeRegisterOrChangeNumberRequestFromSession(session)
|
||||
}
|
||||
|
||||
if inMemoryState.needsToAskForDeviceTransfer {
|
||||
@ -1047,8 +1107,29 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
self.sessionManager.completeSession()
|
||||
}
|
||||
|
||||
private func makeRegisterOrChangeNumberRequestFromSession(
|
||||
_ session: RegistrationSession,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return self.makeRegisterOrChangeNumberRequest(
|
||||
.sessionId(session.id),
|
||||
e164: session.e164
|
||||
).then(on: schedulers.main) { [weak self] response in
|
||||
guard let self = self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.handleCreateAccountResponseFromSession(
|
||||
response,
|
||||
sessionFromBeforeRequest: session,
|
||||
retriesLeft: retriesLeft
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private func handleCreateAccountResponseFromSession(
|
||||
_ response: AccountResponse
|
||||
_ response: AccountResponse,
|
||||
sessionFromBeforeRequest: RegistrationSession,
|
||||
retriesLeft: Int
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
switch response {
|
||||
case .success(let identityResponse):
|
||||
@ -1081,14 +1162,31 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
resetSession()
|
||||
return nextStep()
|
||||
|
||||
case .retryAfter:
|
||||
// TODO[Registration] handle retries, possibly automatically if short enough.
|
||||
case .retryAfter(let timeInterval):
|
||||
if timeInterval < Constants.autoRetryInterval {
|
||||
return Guarantee
|
||||
.after(on: schedulers.sharedBackground, seconds: timeInterval)
|
||||
.then(on: schedulers.sync) { [weak self] in
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.makeRegisterOrChangeNumberRequestFromSession(
|
||||
sessionFromBeforeRequest
|
||||
)
|
||||
}
|
||||
}
|
||||
// TODO[Registration] bubble up the error to the ui properly.
|
||||
return .value(.showGenericError)
|
||||
case .deviceTransferPossible:
|
||||
inMemoryState.needsToAskForDeviceTransfer = true
|
||||
return .value(.transferSelection)
|
||||
case .networkError:
|
||||
// TODO[Registration] handle retries
|
||||
if retriesLeft > 0 {
|
||||
return makeRegisterOrChangeNumberRequestFromSession(
|
||||
sessionFromBeforeRequest,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
@ -1096,7 +1194,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
}
|
||||
|
||||
private func startSession(
|
||||
e164: String
|
||||
e164: String,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return pushRegistrationManager.requestPushToken()
|
||||
.then(on: schedulers.sharedBackground) { [weak self] apnsToken -> Guarantee<RegistrationStep> in
|
||||
@ -1119,11 +1218,27 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
case .invalidArgument:
|
||||
// TODO[Registration] populate error state for phone number entry.
|
||||
return .value(.phoneNumberEntry)
|
||||
case .retryAfter:
|
||||
// TODO[Registration] handle retries, possibly automatically if short enough.
|
||||
case .retryAfter(let timeInterval):
|
||||
if timeInterval < Constants.autoRetryInterval {
|
||||
return Guarantee
|
||||
.after(on: strongSelf.schedulers.sharedBackground, seconds: timeInterval)
|
||||
.then(on: strongSelf.schedulers.sync) { [weak self] in
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.startSession(
|
||||
e164: e164
|
||||
)
|
||||
}
|
||||
}
|
||||
return .value(.phoneNumberEntry)
|
||||
case .networkFailure:
|
||||
// TODO[Registration] handle retries
|
||||
if retriesLeft > 0 {
|
||||
return strongSelf.startSession(
|
||||
e164: e164,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
@ -1134,7 +1249,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
|
||||
private func requestSessionCode(
|
||||
session: RegistrationSession,
|
||||
transport: Registration.CodeTransport
|
||||
transport: Registration.CodeTransport,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return sessionManager.requestVerificationCode(
|
||||
for: session,
|
||||
@ -1174,14 +1290,39 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
case .retryAfterTimeout(let session):
|
||||
// TODO[Registration]: just auto retry if short enough?
|
||||
// Clear the pending code; we want the user to press again
|
||||
// once the timeout expires.
|
||||
self.inMemoryState.pendingCodeTransport = nil
|
||||
self.processSession(session)
|
||||
return self.nextStep()
|
||||
|
||||
let timeInterval: TimeInterval?
|
||||
switch transport {
|
||||
case .sms:
|
||||
timeInterval = session.nextSMS
|
||||
case .voice:
|
||||
timeInterval = session.nextCall
|
||||
}
|
||||
if let timeInterval, timeInterval < Constants.autoRetryInterval {
|
||||
return Guarantee
|
||||
.after(on: self.schedulers.sharedBackground, seconds: timeInterval)
|
||||
.then(on: self.schedulers.sync) { [weak self] in
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.requestSessionCode(
|
||||
session: session,
|
||||
transport: transport
|
||||
)
|
||||
}
|
||||
} else {
|
||||
self.inMemoryState.pendingCodeTransport = nil
|
||||
return self.nextStep()
|
||||
}
|
||||
case .networkFailure:
|
||||
// TODO[Registration] handle retries
|
||||
if retriesLeft > 0 {
|
||||
return self.requestSessionCode(
|
||||
session: session,
|
||||
transport: transport,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
self.inMemoryState.pendingCodeTransport = nil
|
||||
@ -1192,7 +1333,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
|
||||
private func submitCaptchaChallengeFulfillment(
|
||||
session: RegistrationSession,
|
||||
token: String
|
||||
token: String,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return sessionManager.fulfillChallenge(
|
||||
for: session,
|
||||
@ -1227,14 +1369,20 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
case .retryAfterTimeout(let session):
|
||||
// TODO[Registration]: just auto retry if short enough?
|
||||
Logger.error("Should not have to retry a captcha challenge request")
|
||||
// Clear the pending code; we want the user to press again
|
||||
// once the timeout expires.
|
||||
self.inMemoryState.pendingCodeTransport = nil
|
||||
self.processSession(session)
|
||||
return self.nextStep()
|
||||
case .networkFailure:
|
||||
// TODO[Registration] handle retries
|
||||
if retriesLeft > 0 {
|
||||
return self.submitCaptchaChallengeFulfillment(
|
||||
session: session,
|
||||
token: token,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
@ -1244,7 +1392,8 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
|
||||
private func submitSessionCode(
|
||||
session: RegistrationSession,
|
||||
code: String
|
||||
code: String,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
return sessionManager.submitVerificationCode(
|
||||
for: session,
|
||||
@ -1283,11 +1432,30 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
case .retryAfterTimeout(let session):
|
||||
// TODO[Registration]: need to wait out timeout; show error, or retry if short enough.
|
||||
self.processSession(session)
|
||||
if let timeInterval = session.nextVerificationAttempt, timeInterval < Constants.autoRetryInterval {
|
||||
return Guarantee
|
||||
.after(on: self.schedulers.sharedBackground, seconds: timeInterval)
|
||||
.then(on: self.schedulers.sync) { [weak self] in
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
return self.submitSessionCode(
|
||||
session: session,
|
||||
code: code
|
||||
)
|
||||
}
|
||||
}
|
||||
// TODO[Registration]: show some kind of error.
|
||||
return self.nextStep()
|
||||
case .networkFailure:
|
||||
// TODO[Registration] handle retries
|
||||
if retriesLeft > 0 {
|
||||
return self.submitSessionCode(
|
||||
session: session,
|
||||
code: code,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
@ -1359,7 +1527,10 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
return exportAndWipeState(accountIdentity: accountIdentity)
|
||||
}
|
||||
|
||||
private func syncPushTokens(_ accountIdentity: AccountIdentity) -> Guarantee<RegistrationStep> {
|
||||
private func syncPushTokens(
|
||||
_ accountIdentity: AccountIdentity,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
pushRegistrationManager
|
||||
.syncPushTokensForcingUpload(
|
||||
authUsername: accountIdentity.authUsername,
|
||||
@ -1391,13 +1562,25 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
}
|
||||
}
|
||||
return self.nextStep()
|
||||
case .networkError:
|
||||
if retriesLeft > 0 {
|
||||
return self.syncPushTokens(
|
||||
accountIdentity,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private func restoreKBSBackupPostRegistration(pin: String, accountIdentity: AccountIdentity) -> Guarantee<RegistrationStep> {
|
||||
private func restoreKBSBackupPostRegistration(
|
||||
pin: String,
|
||||
accountIdentity: AccountIdentity,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
let backupAuthMethod = KBS.AuthMethod.chatServerAuth(username: accountIdentity.authUsername, password: accountIdentity.authPassword)
|
||||
let authMethod: KBS.AuthMethod
|
||||
if let kbsAuthCredential = inMemoryState.kbsAuthCredentialForPostRegRestoration {
|
||||
@ -1428,13 +1611,26 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
// recover. Keep going like if nothing happened.
|
||||
self.inMemoryState.shouldRestoreKBSMasterKeyAfterRegistration = false
|
||||
return self.nextStep()
|
||||
case .networkError:
|
||||
if retriesLeft > 0 {
|
||||
return self.restoreKBSBackupPostRegistration(
|
||||
pin: pin,
|
||||
accountIdentity: accountIdentity,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
case .genericError:
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private func backupToKBS(pin: String, accountIdentity: AccountIdentity) -> Guarantee<RegistrationStep> {
|
||||
private func backupToKBS(
|
||||
pin: String,
|
||||
accountIdentity: AccountIdentity,
|
||||
retriesLeft: Int = Constants.networkErrorRetries
|
||||
) -> Guarantee<RegistrationStep> {
|
||||
let authMethod: KBS.AuthMethod
|
||||
let backupAuthMethod = KBS.AuthMethod.chatServerAuth(
|
||||
username: accountIdentity.authUsername,
|
||||
@ -1473,8 +1669,14 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
guard let self else {
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
// TODO[Registration]: retry network failures.
|
||||
if case OWSHTTPError.networkFailure = error {
|
||||
if error.isNetworkConnectivityFailure {
|
||||
if retriesLeft > 0 {
|
||||
return self.backupToKBS(
|
||||
pin: pin,
|
||||
accountIdentity: accountIdentity,
|
||||
retriesLeft: retriesLeft - 1
|
||||
)
|
||||
}
|
||||
return .value(.showGenericError)
|
||||
}
|
||||
Logger.error("Failed to back up to KBS with error: \(error)")
|
||||
@ -1658,5 +1860,13 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
|
||||
|
||||
enum Constants {
|
||||
static let persistedStateKey = "state"
|
||||
|
||||
// how many times we will retry network errors.
|
||||
static let networkErrorRetries = 1
|
||||
|
||||
// If a request that can be retried has a timeout below this
|
||||
// threshold, we will auto-retry it.
|
||||
// (e.g. you try sending an sms code and the nextSMS is less than this.)
|
||||
static let autoRetryInterval: TimeInterval = 0.5
|
||||
}
|
||||
}
|
||||
|
||||
@ -492,6 +492,137 @@ public class RegistrationCoordinatorTest: XCTestCase {
|
||||
XCTAssertFalse(kbs.hasMasterKey)
|
||||
}
|
||||
|
||||
func testRegRecoveryPwPath_retryNetworkError() throws {
|
||||
// Set profile info so we skip those steps.
|
||||
setupDefaultAccountAttributes()
|
||||
|
||||
// Set a PIN on disk.
|
||||
ows2FAManagerMock.pinCodeMock = { Stubs.pinCode }
|
||||
|
||||
// Make KBS give us back a reg recovery password.
|
||||
kbs.dataGenerator = {
|
||||
switch $0 {
|
||||
case .registrationRecoveryPassword:
|
||||
return Stubs.regRecoveryPwData
|
||||
case .registrationLock:
|
||||
return Stubs.reglockData
|
||||
default:
|
||||
return nil
|
||||
}
|
||||
}
|
||||
kbs.hasMasterKey = true
|
||||
|
||||
// Run the scheduler for a bit; we don't care about timing these bits.
|
||||
scheduler.start()
|
||||
|
||||
// We haven't set a phone number so it should ask for that.
|
||||
XCTAssertEqual(coordinator.nextStep().value, .phoneNumberEntry)
|
||||
|
||||
// Give it a phone number, which should show the PIN entry step.
|
||||
var nextStep = coordinator.submitE164(Stubs.e164)
|
||||
// Now it should ask for the PIN to confirm the user knows it.
|
||||
XCTAssertEqual(nextStep.value, .pinEntry)
|
||||
|
||||
// Now we want to control timing so we can verify things happened in the right order.
|
||||
scheduler.stop()
|
||||
scheduler.adjustTime(to: 0)
|
||||
|
||||
// Give it the pin code, which should make it try and register.
|
||||
nextStep = coordinator.submitPINCode(Stubs.pinCode)
|
||||
|
||||
let expectedRecoveryPwRequest = RegistrationRequestFactory.createAccountRequest(
|
||||
verificationMethod: .recoveryPassword(Stubs.regRecoveryPw),
|
||||
e164: Stubs.e164,
|
||||
accountAttributes: Stubs.accountAttributes(),
|
||||
skipDeviceTransfer: true
|
||||
)
|
||||
|
||||
// Fail the request at t=2 with a network error.
|
||||
let failResponse = TSRequestOWSURLSessionMock.Response.networkError(url: expectedRecoveryPwRequest.url!)
|
||||
mockURLSession.addResponse(failResponse, atTime: 2, on: scheduler)
|
||||
|
||||
let identityResponse = Stubs.accountIdentityResponse()
|
||||
let authUsername = identityResponse.aci.uuidString
|
||||
var authPassword: String!
|
||||
|
||||
// Once the first request fails, at t=2, it should retry.
|
||||
scheduler.run(atTime: 1) {
|
||||
// Resolve with success at t=3
|
||||
let expectedRequest = RegistrationRequestFactory.createAccountRequest(
|
||||
verificationMethod: .recoveryPassword(Stubs.regRecoveryPw),
|
||||
e164: Stubs.e164,
|
||||
accountAttributes: Stubs.accountAttributes(),
|
||||
skipDeviceTransfer: true
|
||||
)
|
||||
|
||||
self.mockURLSession.addResponse(
|
||||
TSRequestOWSURLSessionMock.Response(
|
||||
matcher: { request in
|
||||
// The password is generated internally by RegistrationCoordinator.
|
||||
// Extract it so we can check that the same password sent to the server
|
||||
// to register is used later for other requests.
|
||||
authPassword = Self.attributesFromCreateAccountRequest(request).authKey
|
||||
return request.url == expectedRequest.url
|
||||
},
|
||||
statusCode: 200,
|
||||
bodyData: try! JSONEncoder().encode(identityResponse)
|
||||
),
|
||||
atTime: 3,
|
||||
on: self.scheduler
|
||||
)
|
||||
}
|
||||
|
||||
// When registered at t=3, it should try and sync push tokens. Succeed at t=4
|
||||
pushRegistrationManagerMock.syncPushTokensForcingUploadMock = { username, password in
|
||||
XCTAssertEqual(self.scheduler.currentTime, 3)
|
||||
XCTAssertEqual(username, authUsername)
|
||||
XCTAssertEqual(password, authPassword)
|
||||
return self.scheduler.guarantee(resolvingWith: .success, atTime: 4)
|
||||
}
|
||||
|
||||
// We haven't done a kbs backup; that should happen at t=4. Succeed at t=5.
|
||||
kbs.generateAndBackupKeysMock = { pin, authMethod, rotateMasterKey in
|
||||
XCTAssertEqual(self.scheduler.currentTime, 4)
|
||||
XCTAssertEqual(pin, Stubs.pinCode)
|
||||
// We don't have a kbs auth credential, it should use chat server creds.
|
||||
XCTAssertEqual(authMethod, .chatServerAuth(username: authUsername, password: authPassword))
|
||||
XCTAssertFalse(rotateMasterKey)
|
||||
self.kbs.hasMasterKey = true
|
||||
return self.scheduler.promise(resolvingWith: (), atTime: 5)
|
||||
}
|
||||
|
||||
// Once we sync push tokens at t=5, we should restore from storage service.
|
||||
// Succeed at t=6.
|
||||
accountManagerMock.performInitialStorageServiceRestoreMock = {
|
||||
XCTAssertEqual(self.scheduler.currentTime, 5)
|
||||
return self.scheduler.promise(resolvingWith: (), atTime: 6)
|
||||
}
|
||||
|
||||
// Once we do the storage service restore at t=6,
|
||||
// we will sync account attributes and then we are finished!
|
||||
let expectedAttributesRequest = RegistrationRequestFactory.updatePrimaryDeviceAccountAttributesRequest(
|
||||
Stubs.accountAttributes(),
|
||||
authUsername: "", // doesn't matter for url matching
|
||||
authPassword: "" // doesn't matter for url matching
|
||||
)
|
||||
self.mockURLSession.addResponse(
|
||||
TSRequestOWSURLSessionMock.Response(
|
||||
matcher: { request in
|
||||
return request.url == expectedAttributesRequest.url
|
||||
},
|
||||
statusCode: 200,
|
||||
bodyData: nil
|
||||
),
|
||||
atTime: 7,
|
||||
on: scheduler
|
||||
)
|
||||
|
||||
scheduler.runUntilIdle()
|
||||
XCTAssertEqual(scheduler.currentTime, 7)
|
||||
|
||||
XCTAssertEqual(nextStep.value, .done)
|
||||
}
|
||||
|
||||
// MARK: - KBS Auth Credential Path
|
||||
|
||||
func testKBSAuthCredentialPath_happyPath() {
|
||||
|
||||
@ -175,8 +175,10 @@ public class KeyBackupService: KeyBackupServiceProtocol {
|
||||
throw KBS.KBSError.invalidPin(triesRemaining: UInt32(remainingAttempts))
|
||||
case .backupMissing:
|
||||
throw KBS.KBSError.backupMissing
|
||||
case .genericError:
|
||||
throw KBS.KBSError.assertion
|
||||
case .networkError(let error):
|
||||
throw error
|
||||
case .genericError(let error):
|
||||
throw error
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -198,7 +200,7 @@ public class KeyBackupService: KeyBackupServiceProtocol {
|
||||
) -> Guarantee<KBS.RestoreKeysResult> {
|
||||
guard let enclave = enclavesToCheck.first else {
|
||||
owsFailDebug("Unexpectedly tried to restore keys with no specified enclaves")
|
||||
return .value(.genericError)
|
||||
return .value(.genericError(KBS.KBSError.assertion))
|
||||
}
|
||||
return restoreKeysAndBackup(
|
||||
pin: pin,
|
||||
@ -206,7 +208,7 @@ public class KeyBackupService: KeyBackupServiceProtocol {
|
||||
enclave: enclave
|
||||
).then(on: schedulers.sync) { result -> Guarantee<KBS.RestoreKeysResult> in
|
||||
switch result {
|
||||
case .success, .invalidPin, .genericError:
|
||||
case .success, .invalidPin, .networkError, .genericError:
|
||||
return .value(result)
|
||||
case .backupMissing:
|
||||
if enclavesToCheck.count > 1 {
|
||||
@ -304,14 +306,17 @@ public class KeyBackupService: KeyBackupServiceProtocol {
|
||||
return .success
|
||||
}
|
||||
.recover(on: schedulers.global()) { error -> Guarantee<KBS.RestoreKeysResult> in
|
||||
if error.isNetworkConnectivityFailure {
|
||||
return .value(.networkError(error))
|
||||
}
|
||||
guard let kbsError = error as? KBS.KBSError else {
|
||||
owsFailDebug("Unexpectedly surfacing a non KBS error \(error)")
|
||||
return .value(.genericError)
|
||||
return .value(.genericError(error))
|
||||
}
|
||||
|
||||
switch kbsError {
|
||||
case .assertion:
|
||||
return .value(.genericError)
|
||||
return .value(.genericError(error))
|
||||
case .invalidPin(let remainingAttempts):
|
||||
return .value(.invalidPin(remainingAttempts: Int(remainingAttempts)))
|
||||
case .backupMissing:
|
||||
|
||||
@ -88,8 +88,9 @@ public enum KBS {
|
||||
// This could mean there was never a backup, or it's been
|
||||
// deleted due to using up all pin attempts.
|
||||
case backupMissing
|
||||
// Network or other issue.
|
||||
case genericError
|
||||
case networkError(Error)
|
||||
// Some other issue.
|
||||
case genericError(Error)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -20,6 +20,8 @@ public class TSRequestOWSURLSessionMock: BaseOWSURLSessionMock {
|
||||
let headers: OWSHttpHeaders
|
||||
let bodyData: Data?
|
||||
|
||||
let error: Error?
|
||||
|
||||
public init(
|
||||
matcher: @escaping (TSRequest) -> Bool,
|
||||
statusCode: Int = 200,
|
||||
@ -30,6 +32,7 @@ public class TSRequestOWSURLSessionMock: BaseOWSURLSessionMock {
|
||||
self.statusCode = statusCode
|
||||
self.headers = headers
|
||||
self.bodyData = bodyData
|
||||
self.error = nil
|
||||
}
|
||||
|
||||
public init(
|
||||
@ -50,6 +53,7 @@ public class TSRequestOWSURLSessionMock: BaseOWSURLSessionMock {
|
||||
self.statusCode = statusCode
|
||||
self.headers = OWSHttpHeaders(httpHeaders: headers, overwriteOnConflict: true)
|
||||
self.bodyData = bodyData
|
||||
self.error = nil
|
||||
}
|
||||
|
||||
public init(
|
||||
@ -65,6 +69,30 @@ public class TSRequestOWSURLSessionMock: BaseOWSURLSessionMock {
|
||||
bodyJson: bodyJson
|
||||
)
|
||||
}
|
||||
|
||||
private init(
|
||||
matcher: @escaping (TSRequest) -> Bool,
|
||||
error: Error
|
||||
) {
|
||||
self.matcher = matcher
|
||||
self.statusCode = 0
|
||||
self.headers = .init()
|
||||
self.bodyData = nil
|
||||
self.error = error
|
||||
}
|
||||
|
||||
public static func networkError(
|
||||
url: URL
|
||||
) -> Self {
|
||||
Self.init(matcher: { $0.url == url }, error: OWSHTTPError.networkFailure(requestUrl: url))
|
||||
}
|
||||
|
||||
public static func networkError(
|
||||
matcher: @escaping (TSRequest) -> Bool,
|
||||
url: URL
|
||||
) -> Self {
|
||||
Self.init(matcher: matcher, error: OWSHTTPError.networkFailure(requestUrl: url))
|
||||
}
|
||||
}
|
||||
|
||||
public var responses = [(Response, Guarantee<Response>)]()
|
||||
@ -113,13 +141,16 @@ public class TSRequestOWSURLSessionMock: BaseOWSURLSessionMock {
|
||||
fatalError("Got a request with no response set up!")
|
||||
}
|
||||
let response = responses.remove(at: responseIndex)
|
||||
return response.1.map(on: SyncScheduler()) {
|
||||
return HTTPResponseImpl(
|
||||
return response.1.then(on: SyncScheduler()) { response throws -> Promise<HTTPResponse> in
|
||||
if let error = response.error {
|
||||
throw error
|
||||
}
|
||||
return .value(HTTPResponseImpl(
|
||||
requestUrl: rawRequest.url!,
|
||||
status: $0.statusCode,
|
||||
headers: $0.headers,
|
||||
bodyData: $0.bodyData
|
||||
)
|
||||
status: response.statusCode,
|
||||
headers: response.headers,
|
||||
bodyData: response.bodyData
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -544,12 +544,12 @@ public class RegistrationSessionManagerImpl: RegistrationSessionManager {
|
||||
)
|
||||
}
|
||||
.recover(on: schedulers.sync) { (error: Error) -> Guarantee<ResponseType> in
|
||||
if error.isNetworkConnectivityFailure {
|
||||
return .value(networkFailureError)
|
||||
}
|
||||
guard let error = error as? OWSHTTPError else {
|
||||
return .value(fallbackError)
|
||||
}
|
||||
if case .networkFailure = error {
|
||||
return .value(networkFailureError)
|
||||
}
|
||||
let response = handler(
|
||||
e164,
|
||||
error.responseStatusCode,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user