Skip to content

Conversation

@jurij-jukic
Copy link

  • handle_http_server_message function has not been tampered with, so there is some redundancy. I.e. the message is being transformed twice, outside and inside the handler. I prefer that the functions handle_xyz_message have the same format. If priority is different, I can trim it down and change the name to e.g. handle_http_server_http_request.

  • There are four cases of messages relevant in this context: local through some app, local through http-server, remote through some app, remote through http-server.

  • When done with kit inject-message, messages will go through http-server:distro:sys. So whether I send a message tagged as remote or as local, they will both end up in the local handler. Not sure if this is correct behavior, but it makes sense given that all calls via the rpc endpoint are expected to be local (afaict), otherwise they would be hitting http endpoints directly.

  • If there is a misformatted HTTP message, we will parse it as a local message, which might be clunky for devs. I can add logging in the local message handler to say something like "If you are sending an HTTP message (rather than local), it might be formatted wrong."

@jurij-jukic jurij-jukic requested a review from nick1udwig October 2, 2025 21:17
src/lib.rs Outdated
if message.is_local() && message.source().process == "http-server:distro:sys" {
handle_http_server_message(&mut state, message);
if let Ok(http_server_request) = serde_json::from_slice::<hyperware_process_lib::http::server::HttpServerRequest>(message.body()) {
handle_http_server_message(&mut state, message);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. While we're here, lets change the handle_http_server_message to accept, instead of a message, the http_server_request that we just parsed (so we don't parse it twice in a row).

We'll probably also need to unpack the blob and add that as an arg as well; not sure if anything else.

@jurij-jukic
Copy link
Author

@nick1udwig done and tested.

Copy link
Member

@nick1udwig nick1udwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work ⚡

@jurij-jukic jurij-jukic merged commit ac24940 into develop Oct 8, 2025
@jurij-jukic jurij-jukic deleted the j/local-msg-handling branch October 8, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants