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

feat: add ws proxy functionality #115

Merged
merged 4 commits into from Apr 11, 2023
Merged

Conversation

Rakowskiii
Copy link
Contributor

@Rakowskiii Rakowskiii commented Apr 5, 2023

Description

Adding handler for websocket proxying /ws
Currently only supporting infura, as pokt doesn't seem to support websockets.
Zksync is deprecating ws https://docs.zksync.io/api/sdk/js/providers/#create-websocket-provider-it-is-deprecated-and-will-be-removed-soon

Resolves #87

How Has This Been Tested?

Tests ran for each connection.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@Rakowskiii Rakowskiii self-assigned this Apr 5, 2023
@Rakowskiii Rakowskiii requested review from a team and xDarksome and removed request for a team and xDarksome April 11, 2023 08:47
}

impl IntoResponse for RpcError {
fn into_response(self) -> axum::response::Response {
error!("{:?}", self);
Copy link
Member

Choose a reason for hiding this comment

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

Is this a recommended way to log errors in axum? I tend to consider logging inside conversion functions to be a bad practice.

Is there some other place we could tap_err or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is recommended way.
My thought process behind this was: tapping in each place separately may lead to some uncovered spots in the future. Logging it here take off the responsibility from future contributors, as they don't need to keep remembering about each and every error they return.

Copy link
Member

Choose a reason for hiding this comment

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

We should be able to do logging via some middleware abstraction of axum, like layer or something, but I don't have a better solution rn. So LGTM for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll drop this as issue, thanks

src/extractors/protocol.rs Outdated Show resolved Hide resolved
src/providers/mod.rs Outdated Show resolved Hide resolved
tests/functional/websocket/infura.rs Show resolved Hide resolved
@xDarksome
Copy link
Member

Also, fix the unused code warning in tests/context/mod.rs, please.

}

#[async_trait]
pub trait RpcWsProvider: Send + Sync + Provider {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Send + Sync + 'static are conventionally being written at the end

}

impl IntoResponse for RpcError {
fn into_response(self) -> axum::response::Response {
error!("{:?}", self);
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to do logging via some middleware abstraction of axum, like layer or something, but I don't have a better solution rn. So LGTM for now

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.

feat: add websocket support
2 participants