Skip to content

feat/RPC-style actions#154

Open
Abdullah-Azbah wants to merge 1 commit intodmotz:mainfrom
Abdullah-Azbah:feat/rpc-style-actions
Open

feat/RPC-style actions#154
Abdullah-Azbah wants to merge 1 commit intodmotz:mainfrom
Abdullah-Azbah:feat/rpc-style-actions

Conversation

@Abdullah-Azbah
Copy link
Copy Markdown

Overview

This PR introduces makeRpc, a request/response abstraction built on top of the existing makeAction primitive. Until now the library only supported fire-and-forget messaging; there was no built-in way to send a message to a peer and await a response. makeRpc fills that gap by layering a lightweight call/response protocol on top of named actions.

Example

Each RPC is registered under a unique name that is shared with actions. When a peer fires an RPC call, the following happen:

  1. A unique id is generated for that call.
  2. A timer is started to insure that calls to dead peers don't hang indefinitely.
  3. The payload is encoded to bytes using msgpackr and sent to the receiving peer.
  4. The receiving peer decodes the message back to an object, applies the handler to the payload received, re-encodes the result and sends it back to the caller.
  5. The caller resolves or rejects its pending promise based on the result it received (or didn't receive).

here is a basic example of things work

const room = joinRoom({ appId: "my-app-id" }, "my-room");

const rpc = room.makeRpc<number, string>(
  "intToStr",
  (n) => {
    // Do validation if needed
    console.log(`got payload: (${n}), converting to string`);
    return n.toString();
  },
  {
    timeout: 100, // <== 100ms timeout
    progress: (p) => console.log(`send progress: ${p}`),
  },
);

const str = await rpc(peerId, 12345); // <== this returns "12345" that was handled by peerId

@rogersanick
Copy link
Copy Markdown

Hey Abdullah, this is great! Just starting to dive into the code, but at a high level I’d consider renaming this to something like makeResponseAction or makeActionWithResponse.

I think introducing a makeRpc method may confuse users. The name suggests a well-defined remote procedure, whereas this abstraction is closer to request/response semantics layered on top of actions. Peers aren’t guaranteed to share a consistent interface, so the interaction is more loosely coupled than what “RPC” typically implies.

Separately, I’d recommend splitting the msgpackr changes into a separate PR. I think we should have a broader discussion before changing the serialization library used in the repo, though this is an awesome idea / change.

@Abdullah-Azbah
Copy link
Copy Markdown
Author

@rogersanick Thank you for your valuable input. Yes, i can see why this name can be misleading.

Regarding the use of msgapckr, i mainly used since trystero doesn't support arbitrary objects. For example, the code below doesn't work as expected. But i do agree with you completely about the seperate PR. I will remove msgpackr and do the renames later today.

const [sendData, getData] = room.makeAction("data");

sendData({
  buffer: new Uint32Array([100, 200, 300])
})

getData((data) => {
  console.log(data.buffer); // <== {buffer: {0: 100, 1: 200, 2: 300} }
  console.log(typeof data.buffer) // <== object, should be Uint32Array
});

One more thing that i thought about last night. Should functions to create separate "caller" and "handler" also be provided? To give some context about my specific use case. I have a bunch of "worker" nodes that run on Nodejs. The main server controlling these nodes is hosted on Cloudflare Workers (wan't the best decision, but its too late now). I needed a way to communicate with these workers without having them constantly pooling the main server. This is what inspired this PR.

Now I'm thinking of the following

// on nodes
defineActionHandler('set-settings', (settings) => {
  // Do things with the setting...

  setNodeSetting(settings) 
  // Maybe return something
  return ...
})

// on admin dashboard (browser)
const setNodeSettings = defineActionCaller('set-settings')

const response = action(peerId, {
  logLevel: 'Debug'
})

In my previous implementation (makeRpc), the handler must be available on both ends. With this, the server can define a handler, an the client, running on the browser, can simply define the caller. makeRpc (or whatever we decide to call it) then will be just a thin wrapper around these two.

Let me know what you think.

@ansarizafar
Copy link
Copy Markdown

I really like this idea its a must have feature and It should be implemented ASAP.

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