smol-rs / polling

Portable interface to epoll, kqueue, event ports, and wepoll
Apache License 2.0
534 stars 65 forks source link

Fix: replace is_connect_failed with is_err #189

Closed irvingoujAtDevolution closed 6 months ago

irvingoujAtDevolution commented 7 months ago

In linux, epoll, EPOLLHUP may happen even if no connection call is made. It would confuse callers for what is actually happening. Replaced is_connect_failed, and we detect if connection failed by using the combination of is_err and is_interrupt, please see the example, tcp_client

irvingoujAtDevolution commented 7 months ago

Unfortunately we can't change is_connect_failed as that would be a breaking change. So we have to keep the name the same there.

I apologize for my previous mistake. I would like to have your opinion on whether it is better to add is_err, mark is_connect_fail as deprecated, or simply keep using the same name but change the implementation.

I would argue that the first approach is better, as it aligns with the semantics it provides.

notgull commented 7 months ago

I apologize for my previous mistake. I would like to have your opinion on whether it is better to add is_err, mark is_connect_fail as deprecated, or simply keep using the same name but change the implementation.

I would argue that the first approach is better, as it aligns with the semantics it provides.

I'd argue that the first would be better.

irvingoujAtDevolution commented 6 months ago

please have a look again, I am not exactly sure if this is how you want it.

irvingoujAtDevolution commented 6 months ago

😢 , @notgull could you please help me look at the wine CI thing? I have no idea how that works

notgull commented 6 months ago

Looks like a spurious error.