search

containerd / ttrpc-rust

Rust implementation of ttrpc (GRPC for low-memory environments)
Apache License 2.0
195 stars 45 forks source link

[Windows] Check for errors when connecting with client #182

Closed jsturtevant closed 1 year ago

jsturtevant commented 1 year ago

The unwrap() on the file when creating the client connection causes a panic. This can happen if the file doesn't exist or there is a small chance when multiple clients connect quickly that the pipe returns 'All pipe instances are busy.' also causing a panic. This allows the caller to handle these errors and retry if necessary (as is the case with the pipe instances being busy).

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (6639bca) 24.39% compared to head (d96bea3) 24.39%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #182 +/- ## ======================================= Coverage 24.39% 24.39% ======================================= Files 17 17 Lines 2529 2529 ======================================= Hits 617 617 Misses 1912 1912 ``` | [Impacted Files](https://codecov.io/gh/containerd/ttrpc-rust/pull/182?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) | Coverage Δ | | |---|---|---| | [src/error.rs](https://codecov.io/gh/containerd/ttrpc-rust/pull/182?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd#diff-c3JjL2Vycm9yLnJz) | `55.55% <0.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jsturtevant commented 1 year ago

@wllenyj @Tim-Zhang friendly ping for a merge.

I think these last couple PR should have stabilized the windows support. If we can get these merged and a release out then I start using this in https://github.com/containerd/rust-extensions/pull/139. If there is any other work before a release please let me know and I can try to help out.