HttpApiClient should be able to handle non-JSON request and response bodies

[gabbifish]

There are some APIs at Cloudflare that do not follow our common use of json for request and response bodies: Two examples of this:

1. Reading a Workers KV value by key returns a raw string body: https://api.cloudflare.com/#workers-kv-namespace-read-key-value-pair
2. Setting a Workers KV key-value pair requires a raw string body: https://api.cloudflare.com/#workers-kv-namespace-write-key-value-pair

Because cloudflare-rs expects to serialize/deserialize JSON for all CF API endpoints, these Workers KV API endpoints are not compatible with this library.

Given that this library surfaces an HttpApiClient, not an HttpJsonApiClient, should this trait be built more extensibly, so the parsing of request bodies is directly tied to the "content-type" header associated with it? Perhaps the default serialization/deserialization will be for json, but if a request/response specifies a specific "content-type" header, the body bytes will be parsed as such.

[adamchalmers]

I agree. JSON is a sensible default, but if an API specifically tells us to parse something as plain-text or octet-stream, we should skip the JSON deserialization and just return a String.

[adamchalmers]

BTW you can send a string body by just defining an Endpoint struct where BodyType = String.

[ashleymichal]

yeah we'll need one where BodyType is a multipart form of a v. specific shape.

[ispivey]

@adamchalmers I'm learning myself a lil' Rust and played around with implementing some new APIs, including Upload a Worker which requires a string body. It seems like if you define an Endpoint struct where BodyType = String as you suggested above, the later call to serde_json::to_string(&body).unwrap() in framework::ApiClient::request will wrap the body in quotes. I made a simple example that illustrates:

1. An endpoint with a BodyType that is a struct that contains a string parameter,
2. An endpoint with a BodyType = String,
3. An endpoint with that simply prints the string body, without calling serde_json::to_string on it.

This results in:

1: {"script":"addEventListener('fetch', event => { event.respondWith(fetch(event.request)) })"}
2: "addEventListener('fetch', event => { event.respondWith(fetch(event.request)) })"
3: addEventListener('fetch', event => { event.respondWith(fetch(event.request)) })

3 is the format we need to send in the request body; it appears setting a BodyType = String is insufficient. If we send 2, the Cloudflare API parses the body as JavaScript, concluding that our script contains only JavaScript string and registers no event listener:

HTTP 400 Bad Request:
Error 10021: No event handlers were registered. This script does nothing.

messages: []
result: null
success: false

I'll PR a suggested implementation to let an Endpoint specify how its body should be serialized. Basically:

• In the framework::endpoint::Endpoint trait, add a serialized_body(&self) -> Option<String> default function which calls serde_json::to_string(&body).unwrap() on the result of body()
• Update framework::ApiClient::request to call serialized_body() instead of calling body() and then calling serde_json::to_string() on the result.
• This preserves the current behavior of existing endpoints, but allows implementations to define their own serialization functions. In this case, the CreateScript endpoint could define a serialized_body() function that skips calling serde_json::to_string().

WDYT?

[sssilver]

Hi all! This subject has been coming up a few times, and at this point I think it'd be good for us to have a technical discussion where we all converge on a single solution.

There are two levels of abstraction from which we can think about this problem:

1. HTTP standard. In the HTTP paradigm, the request and the response determine what format their content is in. This is what the content-type header is for. In theory, the same endpoint can be requested (and served) through different content-types. This is the lower level abstraction
2. Cloudflare API. The reality of our API is that every endpoint supports only a single content-type. This is the higher-level abstraction. Cloudflare API Endpoints should declare how they are serialized into an HTTP request, and how their responses are deserialized. This is the approach I prefer, since it reflects the higher-level reality

The idea is that every endpoint type would specify, declaratively, what serde serializer should be used for their request serialization and response deserialization. Let's discuss this?

[ashleymichal]

Let's discuss this?

shall we open an RFC issue? because item 2 is true ALMOST everywhere. Except the PUT /scripts endpoint for workers, which DOES support two content-types for uploads: application/javascript and multipart/form-data (oddly enough it does not support application/json)

[sssilver]

I was thinking a meeting first.

What's the difference in behavior for the two types of uploads for this particular endpoint?

[ashleymichal]

one includes worker bindings, the other does not. if you use application/javascript, we don't touch your bindings, if you use mulitpart/form-data we update your bindings according to the metadata/files attached in addition to your script. wrangler uses the multipart/form-data content type exclusively, but the dashboard editor makes use of the application/javascript endpoint.