-
Notifications
You must be signed in to change notification settings - Fork 191
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
[Dispatcher] Handle fallback to .org #1002
Conversation
@@ -20,7 +20,8 @@ final class Dispatcher: NSObject, Dispatching { | |||
private let logger: ConsoleLogging | |||
|
|||
private let defaultTimeout: Int = 5 | |||
|
|||
private var fallback = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we describe the purpose of the variable here, so it will be easier to get context later?
@@ -72,10 +73,14 @@ final class Dispatcher: NSObject, Dispatching { | |||
.filter { $0 == .connected } | |||
.setFailureType(to: NetworkError.self) | |||
.timeout(.seconds(defaultTimeout), scheduler: concurrentQueue, customError: { .webSocketNotConnected }) | |||
.sink(receiveCompletion: { result in | |||
.sink(receiveCompletion: { [weak self] result in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couldn't be unowned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want 😁
} | ||
|
||
private func handleFallbackIfNeeded(error: NetworkError) { | ||
if error == .webSocketNotConnected && socket.request.url?.host == NetworkConstants.defaultUrl { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you help me understand the logic? it will fallback if socket did not connect and url is relay.walletconnect.com
?
could the fallback also happen when socket did not connected from different reasons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check for relay.walletconnect.com
is needed to avoid reconnection for the cases when the wallet uses a custom relay server, it's their business then.
Fallback will happen when the connection timeout happens, we just called this error .webSocketNotConnected
. Cuz basically there is no error when you try to reach a resource that is not available, it's thrown by timeout, like you have no connection.
var components = URLComponents() | ||
components.scheme = "wss" | ||
components.host = relayHost | ||
components.host = fallback ? NetworkConstants.fallbackUrl : relayHost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that if a wallet wants to enable fallback, for all it's users regardless of their location the url will be .org
for all of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the wallet doesn't have the option to enable/disable fallback. It happens automatically when the end user can't connect to .com
. The fallback
parameter will be set to true
and used only for the current session (until the app is relaunched).
Description
Resolves # (issue)
How Has This Been Tested?
Due Dilligence