You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The GovernorConfigBuilder is configured to only replenish 1 element of the quota every second per ip address, regardless of the rest_rps variable passed in. The rest_rps variable is being used to set the burst_size per ip address, but assuming the burst_size is exhausted, the effective rate limit of a client node is 1 request per second per ip. My suggested fix would be to use .per_nanosecond((1_000_000_000 / rest_rps) as u64) in place of .per_second(1), as this will replenish the rest_rps fully every second. Bug location: https://github.com/AleoNet/snarkOS/blob/mainnet-staging/node/rest/src/lib.rs#L114
429s (too many requests) (and any other errors from the rate-limiting library) end up being swallowed and returned as 200s. As a patch, I'm going to use this in place of the current error handler:
// Properly return a 429 Too Many Requests error
let error_message = error.to_string();
let mut response = Response::new(error_message.clone().into());
*response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
if error_message.contains("Too Many Requests") {
*response.status_mut() = StatusCode::TOO_MANY_REQUESTS;
}
response
})
fulltimemike
changed the title
[Bug]
[Bug] Rest reqeust per second is actually burst per second, and returns 200s instead of errors
Sep 23, 2024
fulltimemike
changed the title
[Bug] Rest reqeust per second is actually burst per second, and returns 200s instead of errors
[Bug] Rest request per second is actually burst per second, and returns 200s instead of errors
Sep 24, 2024
🐛 Bug Report
GovernorConfigBuilder
is configured to only replenish 1 element of the quota every second per ip address, regardless of therest_rps
variable passed in. Therest_rps
variable is being used to set the burst_size per ip address, but assuming theburst_size
is exhausted, the effective rate limit of a client node is 1 request per second per ip. My suggested fix would be to use.per_nanosecond((1_000_000_000 / rest_rps) as u64)
in place of.per_second(1)
, as this will replenish therest_rps
fully every second. Bug location: https://github.com/AleoNet/snarkOS/blob/mainnet-staging/node/rest/src/lib.rs#L114Location: https://github.com/AleoNet/snarkOS/blob/mainnet-staging/node/rest/src/lib.rs#L116
Steps to Reproduce
Run a local devnet with a low rps, like 2. Hit an endpoint twice, and observe you can only hit the rest endpoint once per second thereafter.
Expected Behavior
Errors should contain error codes, not 200s. Rest rps should be the actual requests per second and not just the burst metric.
Your Environment
The text was updated successfully, but these errors were encountered: