rocicorp / repc

The canonical Replicache client, implemented in Rust.
Other
31 stars 7 forks source link

Remove http_request_info from PullResponse #314

Closed arv closed 3 years ago

arv commented 3 years ago

We still need it in BeginTryPullResponse so we unpack the error.

Towards #290

arv commented 3 years ago

Feels a little bit ugly to use an PullError in Puller::pull and then just unpack it in the caller. An alternative could be to not treat HTTP errors as errors and have Puller::pull return (Option<PullResponse>, HttpRequestInfo)?

arv commented 3 years ago

... or leave the error and start using structured errors all the way up to JS.

phritz commented 3 years ago

Feels a little bit ugly to use an PullError in Puller::pull and then just unpack it in the caller. An alternative could be to not treat HTTP errors as errors and have Puller::pull return (Option, HttpRequestInfo)? ... or leave the error and start using structured errors all the way up to JS.

Yes either of those might be better. The first sounds easier. If you feel like going that route we should probably have the pusher work the same way.

arv commented 3 years ago

I'll do Pusher in a new PR

phritz commented 3 years ago

I'll do Pusher in a new PR

Whoops, ignore my recent comment about fetchpuller :) I was doing reviews in reverse chronological order doh.