-
Notifications
You must be signed in to change notification settings - Fork 81
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
Support clients on Cloudflare workers #577
Comments
Here some probe that they won't implement it |
Same bug applies for the field |
Hey Juan, I've seen this issue before, it's really unfortunate that the Cloudflare implementation raises an error. As a library, it's not really feasible to detect the runtime. But I can also see where they are coming from, and that they want to be very clear about what features are supported. We could avoid the issue for For So I'm afraid we don't have an immediate fix for this. We did just merge a feature that will solve this issue, though: #575 adds fetch API clients and handlers. The following import {createFetchClient} from "@bufbuild/connect/protocol";
import {createTransport} from "@bufbuild/connect/protocol-connect";
const transport = createTransport({
httpClient: createFetchClient(fetch),
baseUrl: "https://demo.connect.build",
// ...
}); We still have to make this more convenient to use, but the next release will give you this option. |
Hi @timostamm, I'll try the new transport as soon it is available, thanks! |
It was just released in v0.8.6! Any feedback is much appreciated, Juan 🙂 |
@timostamm can you build an example of how to implement it, please? And I don't know what values to use there |
Yes, that's part of what I was referring to with "more convenient to use" 🙂 You can use the following options: useBinaryFormat: false,
acceptCompression: [],
compressMinBytes: 0,
interceptors: [],
readMaxBytes: 1024 * 1024, // 1 MiB
writeMaxBytes: 1024 * 1024, // 1 MiB
sendCompression: null, |
Awesome! it worked. Is there a place to follow the development of the more convenient API? |
We don't have a good place for Cloudflare support yet. #550 is specific to handlers. Let's use this issue for the time being 🙂 |
@timostamm It worked well on cloudflare workers but, when running on nodejs, it fails with the following error:
I use nodejs for local development and then deploy to cloudflare workers |
@timostamm Cloudflare workers now have support for TCP sockets, it would be awesome to have native support from connect https://developers.cloudflare.com/workers/runtime-apis/tcp-sockets/ |
@timostamm Just chiming in here to say that I'm also using Connect client on Cloudflare and this workaround is working for me. Don't know if I'm missing out on anything in the default connect client, but so far smoke test is fine. |
@juanpmarin did you find any workaround for this to work locally with node? |
@Arttii yes, I use something like this: import { createConnectTransport } from "@connectrpc/connect-web";
import { createFetchClient } from "@connectrpc/connect/protocol";
import { createTransport } from "@connectrpc/connect/protocol-connect";
export function createConnectFetchTransport(baseUrl: string) {
const local = true; //here you should implement a way of knowing if you're running locally
if (isLocal) {
return createConnectTransport({
baseUrl: baseUrl,
});
} else {
return createTransport({
httpClient: createFetchClient(fetch),
baseUrl: baseUrl,
useBinaryFormat: false,
acceptCompression: [],
compressMinBytes: 0,
interceptors: [],
readMaxBytes: 1024 * 1024,
writeMaxBytes: 1024 * 1024,
sendCompression: null,
});
}
} There is a more appropriate way, that is implementing your own transport using fetch similar to what |
What's the current best practice for running Are there any drawbacks of what I'm doing currently? function fetchProxy(input: RequestInfo | URL, init?: RequestInit): Promise<Response> {
// Create a new init object without mode and credentials
const newInit: RequestInit = {...init};
delete newInit.mode;
delete newInit.credentials;
newInit.redirect = 'manual';
// Call the original fetch with the modified init
return fetch(input, newInit);
}
const transport = createConnectTransport({
baseUrl: 'http://localhost',
fetch: fetchProxy,
}); |
Describe the bug
Hi! Thank you for the development of connect
I'm using connect-web to do some service-to-service communication, the client service is deployed in Cloudflare workers and it appears that Cloudflare workers do not support the
mode
field that connect is using in the request initializerI'm getting the error:
They're probably never implementing that field because it does not make sense in the context of Workers
It would be awesome if you expose some option to unset that field from the request initialization
https://github.com/bufbuild/connect-es/blob/6d97256107d128e48f4e298e0be19522ccc5b658/packages/connect-web/src/connect-transport.ts#L146
To Reproduce
Try to call some connect service from cloudflare workers using connect-web
Environment (please complete the following information):
The text was updated successfully, but these errors were encountered: