funny return

[gilescope]

https://github.com/quietlychris/bissel/blob/8efb2660b521a863c56f58f18e30d9b59d4a2f7e/src/node/active.rs#L49

Where does that return return to? It looks to me like it exits the loop, not just the match. I don't think that's what you wanted (unless I missed something)?

[quietlychris]

It's definitely not what I want! Honestly, that bit of code basically falls into the "make it compile" bucket. I'm planning on doing a complete overhaul of the error handling all the way down through Bissel for the next release, which will involve fixing this, adding custom error types (moving away from dyn Error), and probably adding in a library like failure to help with making the whole area a little nicer to deal with.

Thanks for pointing this out; I'll keep this issue open as a reminder to make sure this line is involved in that process.

[quietlychris]

This has begun to be addressed in 2e2e4e7, but definitely still some work to do on that handling that ack so that it's not just a String return.

[quietlychris]

@gilescope I know it's been ages since you looked at this, but I've been slowly chipping away at trying to improve this library. If you'd be interested/willing in taking a second look at either this particular issue or poke through the code a bit in general to see if there are any areas that could have the data flow or error handling improved, I'd welcome any additional feedback!

[gilescope]

I’m no expert on networking but there’s a fair few block_on - I would question if you could go full async there but I do accept that you want something to provide a bit of back pressure so that you don’t try and start more requests than you can finish. We are all on a journey - pleased to see it’s coming on.

…

On Sat, 13 Jan 2024 at 03:20, chris m ***@***.***> wrote: @gilescope <https://github.com/gilescope> I know it's been ages since you looked at this, but I've been slowly chipping away at trying to improve this library. If you'd be interested/willing in taking a second look at either this particular issue or poke through the code a bit in general to see if there are any areas that could have the data flow or error handling improved, I'd welcome any additional feedback! — Reply to this email directly, view it on GitHub <#24 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGEJCFVQUCLOBTUN4VKGV3YOH4RRAVCNFSM5SC3YKR2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBZGAZDONRTHA3A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[quietlychris]

Thank you :) My understanding from https://tokio.rs/tokio/topics/bridging is that calling block_on is basically necessary for moving the result of an async networking call back into synchronous code. Even in the examples where they start on something like a spawn(), getting the owned data back still involves something like

let handle = tokio::spawn(async {
    networking_call().await
});
// Wait in the interim for the spawned task to finish
thread::sleep(Duration::from_millis(a_while));
// Get the result of the networking call back into the sync code
tokio.block_on(handle)?;

I'm trying to mitigate this by having the default Tokio runtime be multi-threaded, but I'm not sure how else to improve this for a request-reply architecture, aside from dropping Tokio entirely for Node-based networking. If you have any suggestions or ideas of where I could look for alternate options, I'd be happy to investigate!