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

Add middleware support #122

Merged
merged 4 commits into from
Apr 21, 2022
Merged

Conversation

emr
Copy link
Contributor

@emr emr commented Apr 14, 2022

Hello, thank you for this useful library. We were using ethers.js to listen to new logs on blockchain, but due to occasional websocket disconnection, we made research for solutions and found this library and decided to use it. Now we want to add our functionalities to the alchemy-web3 provider. Functionalities like monitoring, caching, I mean, middleware things. To do so, we added middleware support to the library. So now we can monitor our outgoing/incoming ws/http messages and cache them.

Example middleware for monitoring:

  ...
  createJsonRpcSenderMiddleware = (): JsonRpcSenderMiddleware => {
    return (request, next) => {
      const startedTime = Date.now();
      this.emit('request', request);
      return next().then((response) => {
        const executionTime = Date.now() - startedTime;
        this.emit('response', response, executionTime);
        return res;
      });
    };
  };
  ...

For caching:

const methodsNotToCache = ['eth_blockNumber', 'net_version', 'eth_call', 'eth_subscribe'];
const createAlchemyCacheMiddleware = async (cache: CacheInterface) => {
  return (request, next) => {
    if (methodsNotToCache.includes(request.method)) {
      return next();
    }
    const cached = await cache.get([request.method, request.params]);
    if (cached) {
      return {
        ...cached,
        id: request.id, // change id with requested id to treat as if we really went to the provider
                        // and to prevent incompatibility between requested id and responded id
      };
    }
    const response = next();
    await cache.set([request.method, request.params], response);
    return response;
  }
}

I removed some boring logics like checking if a request/response is batch, if it contains ids, etc to keep examples simple.

Usage:

const web3 = createAlchemyWeb3(wsUrl, {
  jsonRpcSenderMiddlewares: [
    monitor.createJsonRpcSenderMiddleware(),
    createAlchemyCacheMiddleware(new InMemoryCache()),
    // other middlewares
    // ...
  ],
});

We tested the feature with these 2 functionalities, it works perfectly. If you accept this PR, we can update the docs with your help.

In the future, maybe you can think of implementing internal caching which can be enabled with the library options.

Thanks!

Copy link
Member

@thebrianchen thebrianchen left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! It's super slick, and I can definitely see how this opens a lot of possibilities!

With the current implementation, calling next() twice in a JsonRpcSenderMiddleware function would result in skipped middlewares functions or an array out of bounds error. Can you update the sendJsonRpcPayload function to include a boolean check that makes sure each middleware function can only call next() a single time?

Thanks!

@emr
Copy link
Contributor Author

emr commented Apr 15, 2022

@thebrianchen nice catch. I created a closure and added a condition to ensure that middlewares can call next only once. But if a middleware calls next twice while sending net_version call, the websocket provider catches the error and reconnects:
https://github.com/alchemyplatform/alchemy-web3/blob/master/src/web3-adapter/webSocketProvider.ts#L223=
To prevent this, we need to catch only network errors and let others be thrown in this catch block.

@thebrianchen thebrianchen requested a review from dphilipson April 15, 2022 22:57
Copy link
Member

@thebrianchen thebrianchen left a comment

Choose a reason for hiding this comment

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

thanks for the quick turn around, looks good to me! just adding @dphilipson as a reviewer for another check

@emr
Copy link
Contributor Author

emr commented Apr 16, 2022

@thebrianchen I was thinking of a case where if we want to implement our custom retry logic with middleware, in that case, we would not be able to call next twice. So, removing the called condition may cover that case. Please don't merge this PR now and let me test this case and update the PR when I'm free.

@emr emr requested a review from thebrianchen April 17, 2022 22:02
@emr
Copy link
Contributor Author

emr commented Apr 17, 2022

@thebrianchen I made the change that allows calling next multiple times as well as not to cause any error.

@dphilipson
Copy link
Contributor

@emr Great work on this- it's a fantastic addition. I'll merge this then publish.

@dphilipson dphilipson merged commit 75da446 into alchemyplatform:master Apr 21, 2022
@dphilipson
Copy link
Contributor

Published in v1.4.0.

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