From 976d29038868e6f7b91b62652805e721c07694af Mon Sep 17 00:00:00 2001 From: Abitofevrything <54505189+abitofevrything@users.noreply.github.com> Date: Sat, 27 Apr 2024 14:34:03 +0200 Subject: [PATCH] Add warning for rate limits (#654) * Add warning for rate limits * Fix HttpHandler tests --- lib/src/client_options.dart | 16 +++++++++++++++- lib/src/http/handler.dart | 30 +++++++++++++++++++++++++++++- test/unit/http/handler_test.dart | 16 ++++++++-------- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lib/src/client_options.dart b/lib/src/client_options.dart index 7627fe449..cf1e92f96 100644 --- a/lib/src/client_options.dart +++ b/lib/src/client_options.dart @@ -33,8 +33,20 @@ abstract class ClientOptions { /// The logger to use for this client. Logger get logger => Logger(loggerName); + /// The threshold after which a warning will be logged if a request is waiting for rate limits. + /// + /// If this value is `null`, no warnings are emitted when a long rate limit is encountered. + /// + /// This value is also used to prevent log spam. Requests will only emit a warning once per [rateLimitWarningThreshold], even if they are rate limited + /// multiple times during that period. + final Duration? rateLimitWarningThreshold; + /// Create a new [ClientOptions]. - const ClientOptions({this.plugins = const [], this.loggerName = 'Nyxx'}); + const ClientOptions({ + this.plugins = const [], + this.loggerName = 'Nyxx', + this.rateLimitWarningThreshold = const Duration(seconds: 10), + }); } /// Options for controlling the behavior of a [NyxxRest] client. @@ -100,6 +112,7 @@ class RestClientOptions extends ClientOptions { const RestClientOptions({ super.plugins, super.loggerName, + super.rateLimitWarningThreshold, this.userCacheConfig = const CacheConfig(), this.channelCacheConfig = const CacheConfig(), this.messageCacheConfig = const CacheConfig(), @@ -136,6 +149,7 @@ class GatewayClientOptions extends RestClientOptions { this.minimumSessionStarts = 10, super.plugins, super.loggerName, + super.rateLimitWarningThreshold, super.userCacheConfig, super.channelCacheConfig, super.messageCacheConfig, diff --git a/lib/src/http/handler.dart b/lib/src/http/handler.dart index b26974284..268baf1c9 100644 --- a/lib/src/http/handler.dart +++ b/lib/src/http/handler.dart @@ -97,7 +97,34 @@ class HttpHandler { /// Create a new [HttpHandler]. /// /// {@macro http_handler} - HttpHandler(this.client); + HttpHandler(this.client) { + if (client.options.rateLimitWarningThreshold case final threshold?) { + onRateLimit.listen((info) { + final (:request, :delay, :isGlobal, :isAnticipated) = info; + final requestStopwatch = _latencyStopwatches[request]; + if (requestStopwatch == null) return; + + final totalDelay = requestStopwatch.elapsed + delay; + + // Prevent warnings being emitted too often. This limits warnings to once per [threshold]. + if (totalDelay.inMicroseconds ~/ threshold.inMicroseconds <= requestStopwatch.elapsedMicroseconds ~/ threshold.inMicroseconds) return; + + if (totalDelay > threshold) { + logger.warning( + '${request.loggingId} has been pending for ${requestStopwatch.elapsed} and will be sent in $delay due to rate limiting.' + ' The request will have been pending for $totalDelay.', + ); + if (isAnticipated) { + logger.info('This is a predicted rate limit and was anticipated based on previous responses'); + } else if (isGlobal) { + logger.info('This is a global rate limit and will apply to all requests for the next $delay'); + } else { + logger.info('This rate limit was returned by the API'); + } + } + }); + } + } /// Send [request] to the API and return the response. /// @@ -278,6 +305,7 @@ class HttpHandler { httpClient.close(); _onRequestController.close(); _onResponseController.close(); + _onRateLimitController.close(); } } diff --git a/test/unit/http/handler_test.dart b/test/unit/http/handler_test.dart index 7eb346e23..f153ffeb0 100644 --- a/test/unit/http/handler_test.dart +++ b/test/unit/http/handler_test.dart @@ -27,9 +27,9 @@ void main() { group('execute', () { test('can make basic requests', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); final interceptor = nock('https://discord.com/api/v${client.apiOptions.apiVersion}').get('/test')..reply(200, jsonEncode({'message': 'success'})); @@ -46,9 +46,9 @@ void main() { test('returns the correct response type', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); final scope = nock('https://discord.com/api/v${client.apiOptions.apiVersion}'); final successInterceptor = scope.get('/succeed')..reply(200, jsonEncode({'message': 'success'})); @@ -70,9 +70,9 @@ void main() { test('executeSafe throws on request failure', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); nock('https://discord.com/api/v${client.apiOptions.apiVersion}') ..get('/succeed').reply(200, jsonEncode({'message': 'success'})) @@ -88,9 +88,9 @@ void main() { group('rate limits', () { test('creates buckets from headers', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); nock('https://discord.com/api/v${client.apiOptions.apiVersion}').get('/test').reply( 200, @@ -114,9 +114,9 @@ void main() { test('hold requests when rate limit might be exceeded', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); nock('https://discord.com/api/v${client.apiOptions.apiVersion}').get('/test').reply( 200, @@ -155,9 +155,9 @@ void main() { test('update on 429 response', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); nock('https://discord.com/api/v${client.apiOptions.apiVersion}').get('/test').reply( 429, @@ -204,9 +204,9 @@ void main() { test('handles global rate limit', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); nock('https://discord.com/api/v${client.apiOptions.apiVersion}').get('/test').reply( 429, @@ -253,9 +253,9 @@ void main() { test('handles batch request rate limits', () async { final client = MockNyxx(); - final handler = HttpHandler(client); when(() => client.apiOptions).thenReturn(RestApiOptions(token: 'test token')); when(() => client.options).thenReturn(RestClientOptions()); + final handler = HttpHandler(client); for (final duration in [Duration.zero, Duration(seconds: 4), Duration(seconds: 9)]) { Timer(duration, () {