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

refactor(torii-server): combine tonic and warp #926

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

kariy
Copy link
Member

@kariy kariy commented Sep 26, 2023

Refactor of the warp server to allow for better request forwarding of gRPC requests. Instead of using warp to combine these two services, this PR uses hyper.

The server accepts the requests and check the base path of the URI, if base_path == '/grpc' , then it strip the base path and forward it to the tonic server. Otherwise, it forwards it to warp.

torii-grpc's server warp route has been removed.

IMPORTANT NOTE

  1. There's a bug in the tonic client where the URI path is not respected in Endpoint, but this issue doesn't exist if torii-client is compiled to wasm32-unknown-unknown. Meaning, if the gRPC requests are sent from non-wasm3-unknown-unknown build, then, the request URI would be localhost:8080/ even though in the client code you have set localhost:8080/grpc.

See: hyperium/tonic#1314

  1. For reasons unknown to me, when sending gRPC requests from insomnia, I keep getting name resolution error:
14 Failed to parse DNS address dns:localhost:8080/grpc

@kariy kariy requested a review from tarrencev September 26, 2023 10:24
@kariy
Copy link
Member Author

kariy commented Sep 26, 2023

pretty janky solution tbh

but it works

@kariy
Copy link
Member Author

kariy commented Sep 26, 2023

I'm not sure how to check if the /graphql endpoint is working correctly or not. I've only tested it by opening the playground.

@broody can you help confirming if it works as expected?

@tarrencev tarrencev merged commit 68b5faa into dojoengine:main Sep 26, 2023
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.

2 participants