hyperlocal icon indicating copy to clipboard operation
hyperlocal copied to clipboard

Cargo release 0.9.0

Open fussybeaver opened this issue 1 year ago • 13 comments

Happy to see this crate integrate with the latest Hyper release in https://github.com/softprops/hyperlocal/pull/65 . This is working well with my testing. Is there any remaining work to publish to crates.io ?

Would be keen to see a 0.9.0 release. Grateful for any time spent on this.

@softprops

fussybeaver avatar Jan 20 '24 12:01 fussybeaver

Just poking here in hopes this simply got buried.

I'd be happy to help with maintenance or anything needed to get a new release out.

jalaziz avatar Mar 07 '24 05:03 jalaziz

Hi there.

I'm testing this and when I run the server and then the client I get Failed to accept connection: hyper::Error(Shutdown, Os { code: 57, kind: NotConnected, message: "Socket is not connected" }).

any ideas @iamjpotts ?

softprops avatar Mar 08 '24 21:03 softprops

Note I do get a listening server and a response on the client. I just didn't understand the semantics of the error reported, if it is an error or not ect

softprops avatar Mar 08 '24 22:03 softprops

Hi there.

I'm testing this and when I run the server and then the client I get Failed to accept connection: hyper::Error(Shutdown, Os { code: 57, kind: NotConnected, message: "Socket is not connected" }).

any ideas @iamjpotts ?

What operating system?

Can you provide a minimum repro code snippet?

iamjpotts avatar Mar 11 '24 14:03 iamjpotts

What operating system?

osx

rustc --version     
rustc 1.76.0 (07dca489a 2024-02-04)

Can you provide a minimum repro code snippet?

this is me running the examples in that live in the repo from the usage docs https://github.com/softprops/hyperlocal?tab=readme-ov-file#usage

softprops avatar Mar 11 '24 14:03 softprops

I am able to reproduce the issue on osx 14 (Sonoma) but not Ubuntu 20.04.

cargo run --features server --example server in one terminal and then cargo run --example client in another terminal. Server outputs the error in your comment once for every client run.

iamjpotts avatar Mar 19 '24 03:03 iamjpotts

If in the example client I add a tokio::time::sleep for two seconds after the while loop and before the Ok(()) then the same error message appears, but it is delayed by two seconds, and appears on the server when the client exits.

There are very few references to Kind::Shutdown in the hyper code base, and the server code that results in the Shutdown Err is called by https://github.com/hyperium/hyper/blob/0013bdda5cd34ed6fca089eceb0133395b7be041/src/proto/h1/dispatch.rs#L135-L136 which is calling trait method Dispatch::recv_msg which has two implementations - one for a client, and one for a server. The server implementation simply returns the Err(e) it is passed, which is what the call site in the hyperlocal server sample receives as hyper::Error(Shutdown...). Unlike the server, the client implementation of recv_msg has several things it tries before returning Err.

For the hyperlocal server example, I am inclined to treat a hyper::Error(Shutdown) response as a success response. Does that seem sensible here @softprops ?

If not, I may be at the limit of my knowledge of hyper for how to resolve this.

iamjpotts avatar Mar 19 '24 03:03 iamjpotts

https://github.com/softprops/hyperlocal/pull/67 treats the disconnect as a success, but I am not sure this is the proper approach.

It turns out that hyper does not expose a way to detect Kind::Shutdown and that PR had to resort to inspecting an inner cause error. It seems suspicious that serve_connection doesn't return on OSX until the client has disconnected.

iamjpotts avatar Mar 19 '24 04:03 iamjpotts

@fussybeaver what are your thoughts on the behavior of serve_connection on OSX in the updated server example?

iamjpotts avatar Mar 22 '24 00:03 iamjpotts

I can reproduce this behaviour on an OSX machine. If I add half_close(true) to the server builder, the connection will be made on OSX though, so perhaps this is just and idiosyncratic behaviour of Tokio's UnixStream implementation on OSX.

https://docs.rs/hyper/latest/hyper/server/conn/http1/struct.Builder.html#method.half_close

fussybeaver avatar Mar 22 '24 08:03 fussybeaver

A similar issue was reported in https://github.com/softprops/hyperlocal/issues/61 for version 0.8.0 (the currently published version of the crate that is several years old).

I suspect, though waiting for confirmation, that the individual who reported the issue in #61 is also on OSX.

This seems to be a behavior of OSX and some other operating systems (excluding Linux and Windows) - that if the client closes the connection, the server will get an ENOTCONN error when trying to shutdown the connection.

A very old Node.js PR mentions this: https://github.com/nodejs/node/commit/d5b32246fb

The ENOTCONN error is coming from this call in std's Unix socket shutdown() call:

cvt(unsafe { libc::shutdown(self.as_raw_fd(), how) })?;

Basically poll_shutdown and related variants pass thru the error returned by the operating system's implementation of libc::shutdown - which on OSX, is an error ENOTCONN if the peer is shutdown.

iamjpotts avatar Apr 01 '24 03:04 iamjpotts

@fussybeaver I did not see any change in behavior on OSX on the server when setting half_close(true) - it was still getting an Err back from serve_connection at the moment the client connection is dropped (closed), as evidenced by Client disconnected in the server output.

When you set half_close(true) on the server in the latest version of main are you getting Client disconnected or Accepted connection (to be distinguished from Accepting connection)?

iamjpotts avatar Apr 01 '24 04:04 iamjpotts

@iamjpotts I realise after adjusting the println statements that it doesn't work with half_close(true), but I do get it to work when we disable keep_alive on the server.

There's a comment in the hyper_util codebase around keep alive and unsafe raw file pointer handling: https://github.com/hyperium/hyper-util/blob/61724d117163adf2195c701fa8e06f5c22c0a64d/src/client/legacy/connect/http.rs#L680-L682

fussybeaver avatar Apr 07 '24 17:04 fussybeaver

I'm currently working on a crate named hyper-client-sockets, that will similar to hyperlocal but will fully support hyper v1 and focuses only on clientside hyper (since serverside is irrelevant since hyper v1) with the support for unix, vsock and firecracker sockets (under features). It'll probably be available on crates.io soon.

Update: https://crates.io/crates/hyper-client-sockets/0.1.0

kanpov avatar Jul 17 '24 16:07 kanpov

I could get behind that.. or if @iamjpotts takes over the regular maintenance/release of this crate, since it works fine.

A stable and compatible connection pooling implementation would be quite useful, as hyper_util's one has issues...

fussybeaver avatar Jul 21 '24 19:07 fussybeaver

I could get behind that.. or if @iamjpotts takes over the regular maintenance/release of this crate, since it works fine.

A stable and compatible connection pooling implementation would be quite useful, as hyper_util's one has issues...

Interesting. What are the issues with hyper_util's Client?

kanpov avatar Jul 21 '24 19:07 kanpov

I could get behind that.. or if @iamjpotts takes over the regular maintenance/release of this crate, since it works fine.

A stable and compatible connection pooling implementation would be quite useful, as hyper_util's one has issues...

Also, I don't see why the server part of hyperlocal shouldn't be removed or at least deprecated. As far as I've seen, it's trivial to bind to unix instead of tcp in hyper v1

kanpov avatar Jul 22 '24 09:07 kanpov

Interesting. What are the issues with hyper_util's Client?

There is/(was?) a race condition in the Hyper connection pool that was carried over from the pre-1.x Hyper versions, possibly there's some movement to fix it lately, but as yet not sure it works, as documented here: https://github.com/hyperium/hyper/issues/2312

As to your later question, I think the crate is mainly on life support and needs a revival / new maintainer

fussybeaver avatar Jul 22 '24 10:07 fussybeaver

I do not have any particular affinity to hyperlocal, and probably would not be interested in being a maintainer.

I thought it would be good to keep maintaining an existing crate if the current maintainer wants to continue shepherding it and publishing releases. It seems ready for a 0.9.0 release now.

If hyperlocal fell out of maintenance and other crate became actively maintained, I would probably switch to the other crate.

iamjpotts avatar Jul 22 '24 13:07 iamjpotts

Hi folks, sorry for the delay. I just pushed up 0.9.0 to crates.io.

softprops avatar Jul 22 '24 14:07 softprops