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

Improve error handling and timeouts for WebSockets #2193

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

bridger
Copy link
Contributor

@bridger bridger commented Feb 23, 2020

Improves error handling on WebSocket connections. Improvements to WebSocket-Kit intercept errors or timeouts at the ChannelHandler level and expose them to the WebSocket class.

Now, when an error occurs (such as a malformed WebSocket frame) the error is available via the closeCode on WebSocket. Example:

func connected(request: Request, connection: WebSocket) {
  _ = connection.onClose.always { (result) in
      let closeReason = connectionInfo.connection.closeCode ?? .unknown(0)
      // Application-specific cleanup code here
  }

}

@tanner0101 tanner0101 added the enhancement New feature or request label Feb 26, 2020
@tanner0101 tanner0101 added this to In Progress in Vapor 4 via automation Feb 26, 2020
@tanner0101
Copy link
Member

This PR relies on vapor/websocket-kit#61 now as vapor/websocket-kit#59 was closed.

@tanner0101
Copy link
Member

I guess I should also add a bump to Package.swift when vapor/websocket-kit#59 is merged and there is a new beta release

Yes, that would be great. The new version number is: 2.0.0-beta.2.5

Could you also update the PR title and body to formatted as release notes? Examples can be seen here: https://github.com/vapor/vapor/releases. Once merged this will be released automatically.

@tanner0101 tanner0101 added the semver-patch Internal changes only label Feb 26, 2020
@bridger bridger changed the title Disable NIO Websocket error handling in favor of WebSocket-Kit error handling Improve error handling and timeouts for WebSockets Feb 27, 2020
@bridger
Copy link
Contributor Author

bridger commented Feb 27, 2020

Thanks for the reviews @tanner0101! I've updated this PR.

Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

Thanks!

@tanner0101 tanner0101 merged commit 638dc58 into vapor:master Feb 27, 2020
Vapor 4 automation moved this from In Progress to Done Feb 27, 2020
@tanner0101
Copy link
Member

These changes are now available in 4.0.0-beta.4.2

pull bot pushed a commit to scope-demo/vapor that referenced this pull request Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver-patch Internal changes only
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants