feat: Don't attempt reconnections when the error is unrecoverable
Fixes #25. Should fix #24.
This commit is contained in:
		
							parent
							
								
									7f294d6632
								
							
						
					
					
						commit
						96d9ce4761
					
				| @ -373,7 +373,7 @@ class XmppConnection { | |||||||
|   /// Called when a stream ending error has occurred |   /// Called when a stream ending error has occurred | ||||||
|   Future<void> handleError(XmppError error) async { |   Future<void> handleError(XmppError error) async { | ||||||
|     _log.severe('handleError called with ${error.toString()}'); |     _log.severe('handleError called with ${error.toString()}'); | ||||||
| 
 |      | ||||||
|     // Whenever we encounter an error that would trigger a reconnection attempt while |     // Whenever we encounter an error that would trigger a reconnection attempt while | ||||||
|     // the connection result is being awaited, don't attempt a reconnection but instead |     // the connection result is being awaited, don't attempt a reconnection but instead | ||||||
|     // try to gracefully disconnect. |     // try to gracefully disconnect. | ||||||
| @ -390,11 +390,18 @@ class XmppConnection { | |||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     if (await _connectivityManager.hasConnection()) { |     if (!error.isRecoverable()) { | ||||||
|  |       // We cannot recover this error | ||||||
|  |       _log.severe('Since a $error is not recoverable, not attempting a reconnection'); | ||||||
|       await _setConnectionState(XmppConnectionState.error); |       await _setConnectionState(XmppConnectionState.error); | ||||||
|     } else { |       await _sendEvent( | ||||||
|       await _setConnectionState(XmppConnectionState.notConnected); |         NonRecoverableErrorEvent(error), | ||||||
|  |       ); | ||||||
|  |       return; | ||||||
|     } |     } | ||||||
|  | 
 | ||||||
|  |     // The error is recoverable | ||||||
|  |     await _setConnectionState(XmppConnectionState.notConnected); | ||||||
|     await _reconnectionPolicy.onFailure(); |     await _reconnectionPolicy.onFailure(); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|  | |||||||
| @ -1,20 +1,37 @@ | |||||||
| import 'package:moxxmpp/src/socket.dart'; | import 'package:moxxmpp/src/socket.dart'; | ||||||
| 
 | 
 | ||||||
| /// An internal error class | /// An internal error class | ||||||
| abstract class XmppError {} | // ignore: one_member_abstracts | ||||||
|  | abstract class XmppError { | ||||||
|  |   /// Return true if we can recover from the error by attempting a reconnection. | ||||||
|  |   bool isRecoverable(); | ||||||
|  | } | ||||||
| 
 | 
 | ||||||
| /// Returned if we could not establish a TCP connection | /// Returned if we could not establish a TCP connection | ||||||
| /// to the server. | /// to the server. | ||||||
| class NoConnectionError extends XmppError {} | class NoConnectionError extends XmppError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
| 
 | 
 | ||||||
| /// Returned if a socket error occured | /// Returned if a socket error occured | ||||||
| class SocketError extends XmppError { | class SocketError extends XmppError { | ||||||
|   SocketError(this.event); |   SocketError(this.event); | ||||||
|   final XmppSocketErrorEvent event; |   final XmppSocketErrorEvent event; | ||||||
|  | 
 | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// Returned if we time out | /// Returned if we time out | ||||||
| class TimeoutError extends XmppError {} | class TimeoutError extends XmppError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
| 
 | 
 | ||||||
| /// Returned if we received a stream error | /// Returned if we received a stream error | ||||||
| class StreamError extends XmppError {} | class StreamError extends XmppError { | ||||||
|  |   // TODO(PapaTutuWawa): Be more precise | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
|  | |||||||
| @ -1,4 +1,5 @@ | |||||||
| import 'package:moxxmpp/src/connection.dart'; | import 'package:moxxmpp/src/connection.dart'; | ||||||
|  | import 'package:moxxmpp/src/errors.dart'; | ||||||
| import 'package:moxxmpp/src/jid.dart'; | import 'package:moxxmpp/src/jid.dart'; | ||||||
| import 'package:moxxmpp/src/managers/data.dart'; | import 'package:moxxmpp/src/managers/data.dart'; | ||||||
| import 'package:moxxmpp/src/roster/roster.dart'; | import 'package:moxxmpp/src/roster/roster.dart'; | ||||||
| @ -236,3 +237,12 @@ class OmemoDeviceListUpdatedEvent extends XmppEvent { | |||||||
|   final JID jid; |   final JID jid; | ||||||
|   final List<int> deviceList; |   final List<int> deviceList; | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | /// Triggered when a reconnection is not performed due to a non-recoverable | ||||||
|  | /// error. | ||||||
|  | class NonRecoverableErrorEvent extends XmppEvent { | ||||||
|  |   NonRecoverableErrorEvent(this.error); | ||||||
|  |    | ||||||
|  |   /// The error in question. | ||||||
|  |   final XmppError error; | ||||||
|  | } | ||||||
|  | |||||||
| @ -8,12 +8,20 @@ import 'package:moxxmpp/src/types/result.dart'; | |||||||
| import 'package:moxxmpp/src/xeps/xep_0198/xep_0198.dart'; | import 'package:moxxmpp/src/xeps/xep_0198/xep_0198.dart'; | ||||||
| import 'package:uuid/uuid.dart'; | import 'package:uuid/uuid.dart'; | ||||||
| 
 | 
 | ||||||
| class ResourceBindingFailedError extends NegotiatorError {} | class ResourceBindingFailedError extends NegotiatorError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
| 
 | 
 | ||||||
|  | /// A negotiator that implements resource binding against a random server-provided | ||||||
|  | /// resource. | ||||||
| class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { | class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { | ||||||
|  |   ResourceBindingNegotiator() : super(0, false, bindXmlns, resourceBindingNegotiator); | ||||||
| 
 | 
 | ||||||
|   ResourceBindingNegotiator() : _requestSent = false, super(0, false, bindXmlns, resourceBindingNegotiator); |   /// Flag indicating the state of the negotiator: | ||||||
|   bool _requestSent; |   /// - True: We sent a binding request | ||||||
|  |   /// - False: We have not yet sent the binding request | ||||||
|  |   bool _requestSent = false; | ||||||
| 
 | 
 | ||||||
|   @override |   @override | ||||||
|   bool matchesFeature(List<XMLNode> features) { |   bool matchesFeature(List<XMLNode> features) { | ||||||
|  | |||||||
| @ -1,3 +1,50 @@ | |||||||
| import 'package:moxxmpp/src/negotiators/negotiator.dart'; | import 'package:moxxmpp/src/negotiators/negotiator.dart'; | ||||||
|  | import 'package:moxxmpp/src/stringxml.dart'; | ||||||
| 
 | 
 | ||||||
| class SaslFailedError extends NegotiatorError {} | abstract class SaslError extends NegotiatorError { | ||||||
|  |   static SaslError fromFailure(XMLNode failure) { | ||||||
|  |     XMLNode? error; | ||||||
|  |     for (final child in failure.children) { | ||||||
|  |       if (child.tag == 'text') continue; | ||||||
|  | 
 | ||||||
|  |       error = child; | ||||||
|  |       break; | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     switch (error?.tag) { | ||||||
|  |       case 'credentials-expired': return SaslCredentialsExpiredError(); | ||||||
|  |       case 'not-authorized': return SaslNotAuthorizedError(); | ||||||
|  |       case 'account-disabled': return SaslAccountDisabledError(); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     return SaslUnspecifiedError(); | ||||||
|  |   } | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | /// Triggered when the server returned us a <not-authorized /> failure during SASL | ||||||
|  | /// (https://xmpp.org/rfcs/rfc6120.html#sasl-errors-not-authorized). | ||||||
|  | class SaslNotAuthorizedError extends SaslError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => false; | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | /// Triggered when the server returned us a <credentials-expired /> failure during SASL | ||||||
|  | /// (https://xmpp.org/rfcs/rfc6120.html#sasl-errors-credentials-expired). | ||||||
|  | class SaslCredentialsExpiredError extends SaslError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => false; | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | /// Triggered when the server returned us a <account-disabled /> failure during SASL | ||||||
|  | /// (https://xmpp.org/rfcs/rfc6120.html#sasl-errors-account-disabled). | ||||||
|  | class SaslAccountDisabledError extends SaslError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => false; | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | /// An unspecified SASL error, i.e. everything not matched by any more precise erorr | ||||||
|  | /// class. | ||||||
|  | class SaslUnspecifiedError extends SaslError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
|  | |||||||
| @ -59,7 +59,9 @@ class SaslPlainNegotiator extends SaslNegotiator { | |||||||
|         // We assume it's a <failure/> |         // We assume it's a <failure/> | ||||||
|         final error = nonza.children.first.tag; |         final error = nonza.children.first.tag; | ||||||
|         await attributes.sendEvent(AuthenticationFailedEvent(error)); |         await attributes.sendEvent(AuthenticationFailedEvent(error)); | ||||||
|         return Result(SaslFailedError()); |         return Result( | ||||||
|  |           SaslError.fromFailure(nonza), | ||||||
|  |         ); | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  | |||||||
| @ -218,7 +218,9 @@ class SaslScramNegotiator extends SaslNegotiator { | |||||||
|           await attributes.sendEvent(AuthenticationFailedEvent(error)); |           await attributes.sendEvent(AuthenticationFailedEvent(error)); | ||||||
| 
 | 
 | ||||||
|           _scramState = ScramState.error; |           _scramState = ScramState.error; | ||||||
|           return Result(SaslFailedError()); |           return Result( | ||||||
|  |             SaslError.fromFailure(nonza), | ||||||
|  |           ); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         final challengeBase64 = nonza.innerText(); |         final challengeBase64 = nonza.innerText(); | ||||||
| @ -236,7 +238,9 @@ class SaslScramNegotiator extends SaslNegotiator { | |||||||
|           final error = nonza.children.first.tag; |           final error = nonza.children.first.tag; | ||||||
|           await attributes.sendEvent(AuthenticationFailedEvent(error)); |           await attributes.sendEvent(AuthenticationFailedEvent(error)); | ||||||
|           _scramState = ScramState.error; |           _scramState = ScramState.error; | ||||||
|           return Result(SaslFailedError()); |           return Result( | ||||||
|  |             SaslError.fromFailure(nonza), | ||||||
|  |           ); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         // NOTE: This assumes that the string is always "v=..." and contains no other parameters |         // NOTE: This assumes that the string is always "v=..." and contains no other parameters | ||||||
| @ -246,13 +250,17 @@ class SaslScramNegotiator extends SaslNegotiator { | |||||||
|           //final error = nonza.children.first.tag; |           //final error = nonza.children.first.tag; | ||||||
|           //attributes.sendEvent(AuthenticationFailedEvent(error)); |           //attributes.sendEvent(AuthenticationFailedEvent(error)); | ||||||
|           _scramState = ScramState.error; |           _scramState = ScramState.error; | ||||||
|           return Result(SaslFailedError()); |           return Result( | ||||||
|  |             SaslError.fromFailure(nonza), | ||||||
|  |           ); | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         await attributes.sendEvent(AuthenticationSuccessEvent()); |         await attributes.sendEvent(AuthenticationSuccessEvent()); | ||||||
|         return const Result(NegotiatorState.done); |         return const Result(NegotiatorState.done); | ||||||
|       case ScramState.error: |       case ScramState.error: | ||||||
|         return Result(SaslFailedError()); |         return Result( | ||||||
|  |           SaslError.fromFailure(nonza), | ||||||
|  |         ); | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|  | |||||||
| @ -10,7 +10,10 @@ enum _StartTlsState { | |||||||
|   requested |   requested | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| class StartTLSFailedError extends NegotiatorError {} | class StartTLSFailedError extends NegotiatorError { | ||||||
|  |   @override | ||||||
|  |   bool isRecoverable() => true; | ||||||
|  | } | ||||||
| 
 | 
 | ||||||
| class StartTLSNonza extends XMLNode { | class StartTLSNonza extends XMLNode { | ||||||
|   StartTLSNonza() : super.xmlns( |   StartTLSNonza() : super.xmlns( | ||||||
| @ -19,15 +22,15 @@ class StartTLSNonza extends XMLNode { | |||||||
|   ); |   ); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | /// A negotiator implementing StartTLS. | ||||||
| class StartTlsNegotiator extends XmppFeatureNegotiatorBase { | class StartTlsNegotiator extends XmppFeatureNegotiatorBase { | ||||||
|    |   StartTlsNegotiator() : super(10, true, startTlsXmlns, startTlsNegotiator); | ||||||
|   StartTlsNegotiator() |  | ||||||
|     : _state = _StartTlsState.ready, |  | ||||||
|       _log = Logger('StartTlsNegotiator'), |  | ||||||
|       super(10, true, startTlsXmlns, startTlsNegotiator); |  | ||||||
|   _StartTlsState _state; |  | ||||||
| 
 | 
 | ||||||
|   final Logger _log; |   /// The state of the negotiator. | ||||||
|  |   _StartTlsState _state = _StartTlsState.ready; | ||||||
|  | 
 | ||||||
|  |   /// Logger. | ||||||
|  |   final Logger _log = Logger('StartTlsNegotiator'); | ||||||
| 
 | 
 | ||||||
|   @override |   @override | ||||||
|   Future<Result<NegotiatorState, NegotiatorError>> negotiate(XMLNode nonza) async { |   Future<Result<NegotiatorState, NegotiatorError>> negotiate(XMLNode nonza) async { | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user