Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: pass timeout arg to server #2979

Merged
merged 2 commits into from
Apr 14, 2020
Merged

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Apr 10, 2020

Refactors how we pass url params to the API, all args are now normalised in one place. If a timeout is passed it is passed on to the server as a url search param.

N.b. this does mean that timeouts are set in the client and on the server so it's not always clear which will expire first, though one will expire.

We do our best to ensure the errors are the same, though when the API is streaming there's no way to tell if the request completed successfully or if the timeout occurred as the timeout message arrives in http trailers.

Also fixes a problem we have, where the README says we support passing searchParams as an additional option:

await ipfs.id({
  searchParams: {
    foo: 'bar'
  }
})

yet this ends up getting sent as:

POST /api/v0/id?search-params==%5Bobject+Object%5D

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified against libp2p delegated routing apis. Options are being properly passed into the url params.

Refactors how we pass url params to the API, all args are now normalised
in one place.  If a timeout is passed it is passed on to the server as
a url search param.

N.b. this does mean that timeouts are set in the client and on the server
so it's not always clear which will expire first, though one will expire.

We do our best to ensure the errors are the same, though when the API
is streaming there's no way to tell if the request completed successfully
or if the timeout occured as the timeout message arrives in http trailers.
@achingbrain achingbrain linked an issue Apr 14, 2020 that may be closed by this pull request
@achingbrain achingbrain merged commit 049f085 into master Apr 14, 2020
@achingbrain achingbrain deleted the fix/honour-server-timeout branch April 14, 2020 21:28
SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
Refactors how we pass url params to the API, all args are now normalised
in one place.  If a timeout is passed it is passed on to the server as
a url search param.

N.b. this does mean that timeouts are set in the client and on the server
so it's not always clear which will expire first, though one will expire.

We do our best to ensure the errors are the same, though when the API
is streaming there's no way to tell if the request completed successfully
or if the timeout occurred as the timeout message arrives in http trailers.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove http client timeout code
2 participants