search

vapor / websocket-kit

WebSocket client library built on SwiftNIO
https://docs.vapor.codes/4.0/advanced/websockets/
MIT License
272 stars 79 forks source link

Pass a copy of the control frame buffer to ping/pong callbacks #116

Closed tkrajacic closed 1 year ago

tkrajacic commented 2 years ago

This PR enables access to the data in the control frames handled via onPing/onPong via a new API with the existing onPing and onPong methods being deprecated.

codecov-commenter commented 2 years ago

Codecov Report

Merging #116 (7a5cb18) into main (2ec1450) will decrease coverage by 1.08%. The diff coverage is 53.57%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #116 +/- ## ========================================== - Coverage 83.93% 82.85% -1.08% ========================================== Files 6 6 Lines 641 659 +18 ========================================== + Hits 538 546 +8 - Misses 103 113 +10 ``` | [Impacted Files](https://app.codecov.io/gh/vapor/websocket-kit/pull/116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor) | Coverage Δ | | |---|---|---| | [...bSocketKit/Concurrency/WebSocket+Concurrency.swift](https://app.codecov.io/gh/vapor/websocket-kit/pull/116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9XZWJTb2NrZXRLaXQvQ29uY3VycmVuY3kvV2ViU29ja2V0K0NvbmN1cnJlbmN5LnN3aWZ0) | `48.80% <26.66%> (-5.36%)` | :arrow_down: | | [Sources/WebSocketKit/WebSocket.swift](https://app.codecov.io/gh/vapor/websocket-kit/pull/116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9XZWJTb2NrZXRLaXQvV2ViU29ja2V0LnN3aWZ0) | `89.04% <84.61%> (+0.30%)` | :arrow_up: |
0xTim commented 2 years ago

This is obviously something that's going to have to wait for the next major version of Vapor. Will leave it open

tkrajacic commented 2 years ago

This is obviously something that's going to have to wait for the next major version of Vapor. Will leave it open

Maybe I can restructure it into an additive change (e.g. have an overload). Would that work then?

0xTim commented 2 years ago

Yeah if you can make it work that would be great

tkrajacic commented 2 years ago

@0xTim now the old API is back. no more version bump needed. there is only new API. I am not sure about the deprecation messages, but the renamed: variant doesn't help, since it only takes the method name of the new variant, and those are the same (these are just overloads). I don't think it's possible to make Xcode fix-up the code with the overload automatically.

0xTim commented 1 year ago

@tkrajacic ok now the Sendable stuff is finally landed we can pull this in. Once you've updated I'll merge it

tkrajacic commented 1 year ago

@0xTim I rebased this morning. What do you want updated?

0xTim commented 1 year ago

@tkrajacic there was a release 10 mins ago to fix the Sendable issues from yesterday that are causing conflicts in the merge

VaporBot commented 1 year ago

These changes are now available in 2.14.0