Comments (9)
Also GitHub's docs are here:
from ky.
If I understand this correctly, it's basically a small optimization where the server can inform the client ahead of time that the next request would receive a 429
response. Thus the client can avoid making one unnecessary request instead of simply reacting to the 429
when it happens. And such optimizations can become meaningful at scale, especially if a service counts a 429
against your rate limit quota.
That's kind of interesting, although I'm not really sure what we would do with this information. It implies that we would have to keep track of state across multiple ky()
calls... yuck. If a request is made after the rate limit is exceeded, would we throw an error or delay it until RateLimit-Reset
? Would ky.extend()
instances share the state with their parent? This also seems prone to a backlog of requests building up and then all being sent at once, which seems counter-productive. A properly designed rate limiter shouldn't even have a counter that resets suddenly, it should take a sampling from a rolling window relative to the current request.
The whole proposal seems poorly designed to me. RateLimit-Limit
, really? It's not even defined as an actual limit:
If the client exceeds that limit, it MAY not be served.
Given the above, plus the fact that it's not widely used beyond a few big names, my position is that until this is standardized, it should be implemented in a service client. See gh-got
, for example, which does expose those headers, though I don't think it actually uses them internally.
from ky.
ky already has this logic as part of the retry
option and it even automatically reads the Retry-After
header, so I believe all those questions are already answered by existing code.
This feature request is only about reading a couple more headers, I think.
from ky.
It's true that we handle Retry-After
, which you would primarily see in a 429
status response.
However, Retry-After
is easy to handle because we can decide what to do and trigger an action immediately when it is received (the action being a fetch delayed by a setTimeout()
call is merely incidental).
We could certainly treat RateLimit-Remaining: 0
the same way, but I think that's wrong (otherwise these headers would provide almost no value).
As far as I can tell, the whole point of the proposal is really to make clients react to RateLimit-Remaining: 0
for status 2xx
responses and mark the next request to be delayed preemptively before a 429
error even occurs. At the moment, Ky doesn't have a mechanism to do that.
I guess we could implement partial support and just handle error responses. Don't all of these APIs return Retry-After
for 4xx
errors, though?
from ky.
the whole point of the proposal is really to make clients react to RateLimit-Remaining: 0 for status 2xx responses
I'll clarify my request by updating the title, that's not what I was suggesting.
Don't all of these APIs return Retry-After for 4xx errors, though?
Not in GitHub's case:
https://github.com/orgs/community/discussions/24760
from ky.
Interesting! To me, that seems like pure laziness on their part. I mean, if RateLimit-Reset
can be calculated, then so can Retry-After
. Just use the spec-compliant header for goodness sake.
At any rate 🙃, I can get behind treating these headers equivalently. Even though I don't think that's what the IETF proposal implies.
We should probably do headers.get('Retry-After') || headers.get('RateLimit-Reset') || headers.get('X-RateLimit-Reset')
to be thorough.
from ky.
I'm currently using the Twitch API which implements RateLimit-Reset
with a unix epoch timestamp instead of Retry-After
.
I think a more general approach would be to allow optional custom parsing with access to the headers that returns the milliseconds to retry after.
something like this:
await ky.get('https://foo.com', {
retry: {
parseRetryHeader: (headers) => {
const ratelimit_reset_epoch = headers.get('Ratelimit-Reset')
return new Date(Number(ratelimit_reset_epoch) * 1000) - new Date()
}
}
})
What do you think? 🤔
from ky.
We already support timestamps for Retry-After
, so it would be supported for RateLimit-Reset
also, without the need for a function option.
from ky.
The standards say that neither we, nor GitHub et al., should be using the X-
prefix for headers.
See: RFC 6648
From MDN:
Custom proprietary headers have historically been used with an X- prefix, but this convention was deprecated in June 2012 because of the inconveniences it caused when nonstandard fields became standard in RFC 6648; others are listed in the IANA HTTP Field Name Registry, whose original content was defined in RFC 4229.
I opened a PR that does support it, though. To be pragmatic.
from ky.
Related Issues (20)
- beforeError hook isn't called on 500 responses HOT 4
- how to send binary data? HOT 1
- Error: unsupported BodyInit type
- retry not working? HOT 1
- HTTPError json as unknown/type HOT 3
- Allow extend() to append to prefixUrl HOT 5
- Refresh token with ky: 2 calls instead of 1 HOT 2
- [TypeError: signal.throwIfAborted is not a function (it is undefined)] in React Native context HOT 5
- feat: consider cookiejar support HOT 4
- Set TimeoutError#cause to be Request or URL HOT 2
- New request in afterResponse hook with formData content HOT 1
- How to download files from external host
- Allow for logging network errors and JSON parse errors HOT 5
- retry and beforeRetry not triggered HOT 7
- Consider extracting the HTTPError logic to an external module HOT 4
- Add generic for HTTPError to type the response HOT 2
- Aggregated error on server side. HOT 1
- [Question] Does it support HTTP/2 ? (compraison with Got) HOT 2
- Change the default retry amount HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ky.