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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 1, 2023
Merged

[Dispatcher] Handle fallback to .org #1002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93e6ef4
Handle fallback to .org
alexander-lsvk Jul 31, 2023
dce1166
PR fixes
alexander-lsvk Aug 1, 2023
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: 1 addition & 1 deletion Example/RelayIntegrationTests/RelayClientEndToEndTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ final class RelayClientEndToEndTests: XCTestCase {
projectId: InputConfig.projectId,
socketAuthenticator: socketAuthenticator
)
let socket = WebSocket(url: urlFactory.create())
let socket = WebSocket(url: urlFactory.create(fallback: false))
let webSocketFactory = WebSocketFactoryMock(webSocket: socket)
let logger = ConsoleLogger(suffix: prefix, loggingLevel: .debug)
let dispatcher = Dispatcher(
Expand Down
26 changes: 22 additions & 4 deletions Sources/WalletConnectRelay/Dispatching.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ final class Dispatcher: NSObject, Dispatching {
private let logger: ConsoleLogging

private let defaultTimeout: Int = 5

/// The property is used to determine whether relay.walletconnect.org will be used
/// in case relay.walletconnect.com doesn't respond for some reason (most likely due to being blocked in the user's location).
private var fallback = false
Copy link
Contributor

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?


private let socketConnectionStatusPublisherSubject = CurrentValueSubject<SocketConnectionStatus, Never>(.disconnected)

var socketConnectionStatusPublisher: AnyPublisher<SocketConnectionStatus, Never> {
Expand All @@ -38,7 +41,7 @@ final class Dispatcher: NSObject, Dispatching {
self.relayUrlFactory = relayUrlFactory
self.logger = logger

let socket = socketFactory.create(with: relayUrlFactory.create())
let socket = socketFactory.create(with: relayUrlFactory.create(fallback: fallback))
socket.request.addValue(EnvironmentInfo.userAgent, forHTTPHeaderField: "User-Agent")
self.socket = socket

Expand Down Expand Up @@ -72,10 +75,11 @@ 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: { [unowned self] result in
switch result {
case .failure(let error):
cancellable?.cancel()
self.handleFallbackIfNeeded(error: error)
completion(error)
case .finished: break
}
Expand Down Expand Up @@ -104,7 +108,10 @@ final class Dispatcher: NSObject, Dispatching {
func disconnect(closeCode: URLSessionWebSocketTask.CloseCode) throws {
try socketConnectionHandler.handleDisconnect(closeCode: closeCode)
}
}

// MARK: - Private functions
extension Dispatcher {
private func setUpWebSocketSession() {
socket.onText = { [unowned self] in
self.onMessage?($0)
Expand All @@ -118,11 +125,22 @@ final class Dispatcher: NSObject, Dispatching {
socket.onDisconnect = { [unowned self] error in
self.socketConnectionStatusPublisherSubject.send(.disconnected)
if error != nil {
self.socket.request.url = relayUrlFactory.create()
self.socket.request.url = relayUrlFactory.create(fallback: fallback)
}
Task(priority: .high) {
await self.socketConnectionHandler.handleDisconnection()
}
}
}

private func handleFallbackIfNeeded(error: NetworkError) {
if error == .webSocketNotConnected && socket.request.url?.host == NetworkConstants.defaultUrl {
Copy link
Contributor

@llbartekll llbartekll Jul 31, 2023 •
edited
Loading

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?

Copy link
Contributor Author

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.

logger.debug("[WebSocket] - Fallback to \(NetworkConstants.fallbackUrl)")
fallback = true
socket.request.url = relayUrlFactory.create(fallback: fallback)
Task(priority: .high) {
await self.socketConnectionHandler.handleDisconnection()
}
}
}
}
4 changes: 4 additions & 0 deletions Sources/WalletConnectRelay/Misc/NetworkConstants.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enum NetworkConstants {
static var defaultUrl = "relay.walletconnect.com"
static var fallbackUrl = "relay.walletconnect.org"
}
11 changes: 10 additions & 1 deletion Sources/WalletConnectRelay/Misc/NetworkError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import Foundation

enum NetworkError: Error {
enum NetworkError: Error, Equatable {
case webSocketNotConnected
case sendMessageFailed(Error)
case receiveMessageFailure(Error)

static func == (lhs: NetworkError, rhs: NetworkError) -> Bool {
switch (lhs, rhs) {
case (.webSocketNotConnected, .webSocketNotConnected): return true
case (.sendMessageFailed, .sendMessageFailed): return true
case (.receiveMessageFailure, .receiveMessageFailure): return true
default: return false
}
}
}

extension NetworkError: LocalizedError {
Expand Down
6 changes: 3 additions & 3 deletions Sources/WalletConnectRelay/RelayURLFactory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ struct RelayUrlFactory {
private let relayHost: String
private let projectId: String
private let socketAuthenticator: ClientIdAuthenticating

init(
relayHost: String,
projectId: String,
Expand All @@ -15,10 +15,10 @@ struct RelayUrlFactory {
self.socketAuthenticator = socketAuthenticator
}

func create() -> URL {
func create(fallback: Bool) -> URL {
var components = URLComponents()
components.scheme = "wss"
components.host = relayHost
components.host = fallback ? NetworkConstants.fallbackUrl : relayHost
Copy link
Contributor

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?

Copy link
Contributor Author

@alexander-lsvk alexander-lsvk Aug 1, 2023 •
edited
Loading

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).

components.queryItems = [
URLQueryItem(name: "projectId", value: projectId)
]
Expand Down