Upgrade HTTP request calls from `query` to `update` (upon canister's request)

paulyoung · September 4, 2021, 6:35pm

(Copied from megathread)

I propose that my PR to upgrade HTTP request calls from query to update (upon canister’s request) be considered for merging, with some feedback about anything that may be blocking it.

github.com/dfinity/agent-rs

feat: upgrade HTTP request calls from query to update upon canister's request

dfinity:main ← paulyoung:paulyoung/http-request

opened 01:45AM - 28 May 21 UTC

paulyoung

+77 -21

Edit: the code in this PR no longer infers when to make an update call, it does …it upon the canister’s request. *** This is a first pass at inferring when to make an update call instead of a query call based on HTTP request methods. It's based on [this discussion thread](https://forum.dfinity.org/t/feature-request-map-appropriate-http-request-methods-to-update-calls/4303?u=paulyoung) in the developer forum. Thanks to @nomeata for some feedback and help getting to this point. I think this could be made a lot more configurable (perhaps via a file that is read when `icx-proxy` starts which maps methods/routes to functions) but what's here suits my needs for now so I thought I'd share it in case there's any interest in merging. Thanks in advance for considering this change. *** ```typescript import Text "mo:base/Text"; actor { type HeaderField = (Text, Text); type Token = {}; type StreamingCallbackHttpResponse = { body : Blob; token : Token; }; type StreamingStrategy = { #Callback : { callback : shared Token -> async StreamingCallbackHttpResponse; token : Token; }; }; type HttpRequest = { method : Text; url : Text; headers : [HeaderField]; body : Blob; }; type HttpResponse = { status_code : Nat16; headers : [HeaderField]; body : Blob; streaming_strategy : ?StreamingStrategy; }; public query func http_request(request : HttpRequest) : async HttpResponse { { status_code = 200; headers = []; body = Text.encodeUtf8("Response to " # request.method # " request (query)"); streaming_strategy = null; }; }; public shared func http_request_update(request : HttpRequest) : async HttpResponse { { status_code = 200; headers = []; body = Text.encodeUtf8("Response to " # request.method # " request (update)"); streaming_strategy = null; }; }; }; ``` ``` $ icx-proxy --fetch-root-key version: 0.2.1 May 27 18:24:15.771 INFO Log Level: INFO May 27 18:24:15.797 INFO Starting server. Listening on http://127.0.0.1:3000/ ``` ``` $ curl -v -X GET -H "Content-type: application/json" -H "Accept: application/json" -d '{}' 'http://localhost:3000?canisterId=rrkah-fqaaa-aaaaa-aaaaq-cai' * Trying 127.0.0.1... * TCP_NODELAY set * Connected to localhost (127.0.0.1) port 3000 (#0) > GET /?canisterId=rrkah-fqaaa-aaaaa-aaaaq-cai HTTP/1.1 > Host: localhost:3000 > User-Agent: curl/7.64.1 > Content-type: application/json > Accept: application/json > Content-Length: 61 > * upload completely sent off: 61 out of 61 bytes < HTTP/1.1 200 OK < content-length: 31 < date: Fri, 28 May 2021 01:27:24 GMT < * Connection #0 to host localhost left intact Response to GET request (query)* Closing connection 0 ``` ``` $ curl -v -X POST -H "Content-type: application/json" -H "Accept: application/json" -d '{}' 'http://localhost:3000?canisterId=rrkah-fqaaa-aaaaa-aaaaq-cai' Note: Unnecessary use of -X or --request, POST is already inferred. * Trying 127.0.0.1... * TCP_NODELAY set * Connected to localhost (127.0.0.1) port 3000 (#0) > POST /?canisterId=rrkah-fqaaa-aaaaa-aaaaq-cai HTTP/1.1 > Host: localhost:3000 > User-Agent: curl/7.64.1 > Content-type: application/json > Accept: application/json > Content-Length: 61 > * upload completely sent off: 61 out of 61 bytes < HTTP/1.1 200 OK < content-length: 33 < date: Fri, 28 May 2021 01:27:41 GMT < * Connection #0 to host localhost left intact Response to POST request (update)* Closing connection 0 ```

levi · September 5, 2021, 7:34pm

Yes, I think that dominics main claim on twitter is that the internet-computer can serve web directly without the need for untrustworthy front-ends hosted on amazon servers, well making a http-request-call into the IC as a query call is the SAME THING AS HOSTING A FRONT-END ON AMAZON-SERVERS! How is the claim that this ic-blockchain can serve web true if the blockchain is not serving any web because query calls dont go through a blockchain.

nomeata · September 5, 2021, 8:22pm

Better let’s not get distracted by tweets here and rather discuss the technicalities of changing the actual status quo

levi · September 5, 2021, 9:56pm

What are the technicalities? (Open question)

With the canister paying for itself, the more want there is for the update-calls the more the network grows.

paulyoung · September 6, 2021, 4:39am

paulyoung · September 6, 2021, 4:55am

@nomeata, what changes are required to the service worker?

I’ve been using the branch from the PR I linked above and hadn’t considered needing to change the service worker.

jzxchiang · September 6, 2021, 7:15am

Just to confirm, this PR does not support streaming uploads, right?

For example, to upload an asset >2 MB in size.

levi · September 6, 2021, 9:26pm

@jzxchiang More than 2mb calls is a separate thing, this is giving a canister (or the writer of a canister somehow) the choice to make browser standard http_request calls into an update-call that goes through the full-consensus.

paulyoung · September 6, 2021, 11:33pm

@jzxchiang my PR doesn’t change anything about streaming. If streaming uploads were/are implemented separately then this may work for those too.

jzxchiang · September 6, 2021, 11:52pm

Yeah, but there needs to be custom support for streaming uploads, just like there was custom support for streaming downloads.

In other words, the HttpRequest message probably needs to include a StreamingStrategy, so that icx-proxy can repeatedly call some callback method with new data that it receives via the stream from the client.

But yeah, as you said, that would be a different changelist.

jzxchiang · September 6, 2021, 11:53pm

It applies if you want to upload (or POST) a 10 MB file, for example. See my response above.

harrison · September 15, 2021, 1:21pm

We (Fleek) would really appreciate this feature as well. We need it for several of our products.

levi · September 22, 2021, 12:55pm

@diegop hey can we get a word on this status? What is the thinking of the dfinity-team behind this restriction?

paulyoung · September 23, 2021, 1:31am

My pull request was approved this morning. I’m just waiting on some feedback on how to address the last remaining linter error.

diegop · September 23, 2021, 3:24am

Yes we reviewed yesterday and was merged in today.

But I think Paul beat me to the punch.

paulyoung · September 23, 2021, 5:01am

Not merged yet, but soon hopefully.

alexander · September 23, 2021, 6:42am

Will it be possible to do the same at the canister level?

nomeata · September 23, 2021, 7:49am

Is this live?
(Scnr)

levi · September 23, 2021, 8:00pm

@diegop Awesome! Gratitude.
I will test this out.
Someone ping when this is live.

diegop · October 22, 2021, 10:46pm

Following up: did this get merged, @paulyoung ? if not, let me know so i can follow up internally.

Topic		Replies	Views
How do some canisters handle http requests Developers	2	1998	January 24, 2022
Is it possible to build a REST API in a Canister? Getting Started	10	2894	May 5, 2022
Feature request: map appropriate HTTP request methods to update calls Developers	56	6052	April 22, 2022
Improving how you upgrade http_request calls General	3	351	May 8, 2023
Calling http_request of another canister Developers	3	1133	March 2, 2022

Upgrade HTTP request calls from `query` to `update` (upon canister's request)

Related topics