Fix tests in: feat_add_features_for_other_tls_types #15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the async client connects to the server, it spawns a new thread to
handle the arriving messages asynchronously, which is immediately
detached with no chance of awaiting its completion.
For long-running programs (e.g. a client program that never really disconnects
from the server) this can be problematic, as unexpected stream completions
would go unnoticed. This may happen if the underlying tungstenite websocket
shuts down ('Received close frame: None') but there are no engineio/socketio
close frames. Hence, since the stream terminates, the message handling task
stops without a Close or Error event being fired.
Thus, we now fire an additional Event::Close when the stream terminates,
to signal the (potentially unexpected) close to the user.
Previously, when we emitted an Event::Close, we didn't specify a reason for why this event was emitted. When implementing the close notification on unexpected transport closes, we started sending 'transport close' as the payload for the close event. We now decided to do this more consistently and added a close reason when emitting the Event::Close, so that the client better knows why this event was emitted.
Bumps the patches group with 7 updates in the / directory: | Package | From | To | | --- | --- | --- | | [base64](https://github.com/marshallpierce/rust-base64) | `0.22.0` | `0.22.1` | | [serde](https://github.com/serde-rs/serde) | `1.0.209` | `1.0.215` | | [serde_json](https://github.com/serde-rs/json) | `1.0.127` | `1.0.133` | | [futures-util](https://github.com/rust-lang/futures-rs) | `0.3.30` | `0.3.31` | | [async-trait](https://github.com/dtolnay/async-trait) | `0.1.81` | `0.1.83` | | [async-stream](https://github.com/tokio-rs/async-stream) | `0.3.5` | `0.3.6` | | [url](https://github.com/servo/rust-url) | `2.5.2` | `2.5.4` | Updates `base64` from 0.22.0 to 0.22.1 - [Changelog](https://github.com/marshallpierce/rust-base64/blob/master/RELEASE-NOTES.md) - [Commits](marshallpierce/rust-base64@v0.22.0...v0.22.1) Updates `serde` from 1.0.209 to 1.0.215 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.209...v1.0.215) Updates `serde_json` from 1.0.127 to 1.0.133 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.127...v1.0.133) Updates `futures-util` from 0.3.30 to 0.3.31 - [Release notes](https://github.com/rust-lang/futures-rs/releases) - [Changelog](https://github.com/rust-lang/futures-rs/blob/master/CHANGELOG.md) - [Commits](rust-lang/futures-rs@0.3.30...0.3.31) Updates `async-trait` from 0.1.81 to 0.1.83 - [Release notes](https://github.com/dtolnay/async-trait/releases) - [Commits](dtolnay/async-trait@0.1.81...0.1.83) Updates `async-stream` from 0.3.5 to 0.3.6 - [Release notes](https://github.com/tokio-rs/async-stream/releases) - [Commits](tokio-rs/async-stream@v0.3.5...v0.3.6) Updates `url` from 2.5.2 to 2.5.4 - [Release notes](https://github.com/servo/rust-url/releases) - [Commits](servo/rust-url@v2.5.2...v2.5.4) --- updated-dependencies: - dependency-name: base64 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: serde_json dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: futures-util dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: async-trait dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: async-stream dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches - dependency-name: url dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patches ... Signed-off-by: dependabot[bot] <[email protected]>
Turned out to be rather easy due to all tls being handled by our dependencies. Don't love the _fallback-tls feature, however with it we can use cargo-all-features, which helps reduce feature related bugs. Downside of cargo-all-features is it takes a long time to execute. Fixes: 1c3t3a#407, 1c3t3a#366
There's still an error with ci. engineio/src/lib.rs/test/tls_connector doesn't function the same as native-tls one (as it properly validates hostnames) Needs further investigation as to why the hostname shows as invalid in ci. If we no longer need that, then this PR can be merged.
NXD-0: Update webpki due to vulnerabilities
…s-types' into tyler/feat-add-features-for-other-tls-types
|
@ctrlaltf24 do you want to move forward with this pr, or should I open a new one again the main repo? |
|
Let's do a new PR on the main repo, the old PR doesn't have much, feel free to steal anything you want from the description! Love to see movement on this! Happy to review |
|
Ok, I'll reference your original pr and hopefully we can get some movement with the tests being fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch is up to date with the master, so some of these diffs should go away if your branch is updated.
I updated the docs so that they show examples for both tls backends and compile and run successfully no matter which features are enabled. (That's the reason for embedding doc strings instead of using
///like usual.)I also had to upgrade rustls and tungstenite to get all the dependencies to work out.
The issue with the tests was something with how the certs were generated. It took me a day and a half to figure out that no valid server names were being allowed from the root cert or the server cert. I copied some code that
tokio-rustlsuses to do their tests and everything just worked (for both native-tls and rustls backends), so there was something missing there. This also removes the dependency on having openssl available, so makes developing locally that much easier.