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

[Relay] Socket reconnection #601

Merged
merged 4 commits into from
Nov 23, 2022
Merged

Conversation

flypaper0
Copy link
Contributor

Description

AutomaticConnectionHandler:

  • reconnect on every disconnect in foreground

ManualConnectionHandler:

  • no reconnections

Resolves #576

Due Dilligence

  • Breaking change
  • Requires a documentation update

@flypaper0 flypaper0 changed the base branch from main to develop November 22, 2022 11:04
@arein arein added the accepted label Nov 22, 2022
@flypaper0 flypaper0 force-pushed the feature/socket-reconnection-#576 branch from a431ce7 to 0434e31 Compare November 22, 2022 11:06
@flypaper0 flypaper0 marked this pull request as ready for review November 22, 2022 11:07
Copy link
Contributor

@llbartekll llbartekll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good but this change definitely requires thorough manual testing

appStateObserver.currentState = .foreground
XCTAssertTrue(webSocketSession.isConnected)
webSocketSession.disconnect()
sut.handleDisconnection()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this function be triggered automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's calling in Dispatcher. And here is only AutomaticSocketConnectionHandler tests

socket.disconnect()
}

private func reconnect() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reconnectIfNeeded() ?

@flypaper0 flypaper0 merged commit df9bc93 into develop Nov 23, 2022
@flypaper0 flypaper0 deleted the feature/socket-reconnection-#576 branch November 23, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Relay] WebScoket reconnection for Manual/Automatic handlers
3 participants