Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace JSON-RPC callback with a Promise-based API #2528

Closed
tomaka opened this issue Jul 19, 2022 · 0 comments · Fixed by #2778
Closed

Replace JSON-RPC callback with a Promise-based API #2528

tomaka opened this issue Jul 19, 2022 · 0 comments · Fixed by #2778

Comments

@tomaka
Copy link
Contributor

tomaka commented Jul 19, 2022

Instead of having a callback, the user should manually call a method to retrieve the next JSON-RPC response.
This would make it possible to have back-pressure on the JSON-RPC server.

@mergify mergify bot closed this as completed in #2778 Sep 28, 2022
mergify bot added a commit that referenced this issue Sep 28, 2022
…esponse (#2778)

Close #2528

This PR is a major change in the public API of the JS library, as such I
will bump to 0.7.0 at the next release.

Instead of passing a callback when adding a chain where the JSON-RPC
responses are sent, you are now supposed to call `nextJsonRpcResponse`
on the chain.
This asynchronous function waits (if necessary) for a JSON-RPC response
to be available then returns it.

In addition to this, `sendJsonRpc` now returns an error if the request
is malformed or if the queue is full.

This change in API has several major advantages:

- It removes an ambiguity about what to do if the `jsonRpcCallback`
throws an exception.
- It is now possible to back-pressure the client by not calling
`nextJsonRpcResponse`. This will cause the responses to pile up in the
buffer, until a point where `sendJsonRpc` will return an error.
- It is overall easier to use the API because you no longer have to
create things ahead of time (before the chain is even created) in order
to pass them to the callback. See the tests that this PR rewrites for
example.
- Smoldot crashes are now reported faster, because `nextJsonRpcResponse`
will throw an exception in case of a smoldot crash, while before the
callback was simply not called and you had to detect crashes through
`sendJsonRpc`.

## Migration

It is always possible to migrate to the new API.

If your code looks like this:

```js
const chain = await client.addChain({ jsonRpcCallback, /* other options */ ... });
```

You can rewrite it like this:

```js
const chain = await client.addChain({ /* other options */ ... });

(async () => {
    try {
        while(true) {
            const response = await chain.nextJsonRpcResponse();
            /* do whatever you want here */
        }
    } catch(_error) {
        // Will happen when the chain is removed or the client terminated.
    }
})()
```

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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 a pull request may close this issue.

1 participant