Migrate to null safety (#17)

Named arguments are non-nullable with a default instead of a null
default. This means that code which was passing null through instead of
omitting the argument will be broken, even in unsound mode. I don't
expect any code was doing this, and this API is not one that should need
to be wrapped with argument forwarding.

Author: natebosch

.github/workflows/test-package.yml

@@ -20,7 +20,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        sdk: [dev]
+        sdk: [2.12.0, dev]
     steps:
       - uses: actions/checkout@v2
       - uses: dart-lang/[email protected]
@@ -31,14 +31,17 @@ jobs:
         run: dart pub get
       - name: Check formatting
         run: dart format --output=none --set-exit-if-changed .
-        if: always() && steps.install.outcome == 'success'
+        if: matrix.sdk == 'dev' && steps.install.outcome == 'success'
       - name: Analyze code
         run: dart analyze --fatal-infos
-        if: always() && steps.install.outcome == 'success'
+        if: matrix.sdk == 'dev' && steps.install.outcome == 'success'
+      - name: Analyze code
+        run: dart analyze --fatal-infos
+        if: matrix.sdk != 'dev' && steps.install.outcome == 'success'
 
   # Run tests on a matrix consisting of two dimensions:
   # 1. OS: ubuntu-latest, (macos-latest, windows-latest)
-  # 2. release channel: dev
+  # 2. release channel: oldest stable, dev
   test:
     needs: analyze
     runs-on: ${{ matrix.os }}
@@ -47,7 +50,7 @@ jobs:
       matrix:
         # Add macos-latest and/or windows-latest if relevant for this package.
         os: [ubuntu-latest]
-        sdk: [dev]
+        sdk: [2.12.0, dev]
     steps:
       - uses: actions/checkout@v2
       - uses: dart-lang/[email protected]
@@ -62,30 +65,3 @@ jobs:
       - name: Run Chrome tests
         run: dart test --platform chrome
         if: always() && steps.install.outcome == 'success'
-
-  # Run tests on a matrix consisting of two dimensions:
-  # 1. OS: ubuntu-latest, (macos-latest, windows-latest)
-  # 2. release: 2.1.0
-  test-legacy-sdk:
-    needs: analyze
-    runs-on: ${{ matrix.os }}
-    strategy:
-      fail-fast: false
-      matrix:
-        # Add macos-latest and/or windows-latest if relevant for this package.
-        os: [ubuntu-latest]
-        sdk: [2.1.0]
-    steps:
-      - uses: actions/checkout@v2
-      - uses: dart-lang/[email protected]
-        with:
-          sdk: ${{ matrix.sdk }}
-      - id: install
-        name: Install dependencies
-        run: pub get
-      - name: Run VM tests
-        run: pub run test --platform vm
-        if: always() && steps.install.outcome == 'success'
-      - name: Run Chrome tests
-        run: pub run test --platform chrome
-        if: always() && steps.install.outcome == 'success'

CHANGELOG.md

@@ -1,7 +1,9 @@
-## 0.1.2
+## 0.2.0-dev
 
+* Migrate to null safety.
 * Fix a number of lints affecting package maintenance score.
-* Update minimum Dart SDK to `2.1.0`.
+* **BREAKING** `null` may not be passed for most named arguments, instead the
+  argument must be omitted.
 
 ## 0.1.1+3
 

example/example.dart

@@ -8,7 +8,7 @@ import 'package:http_retry/http_retry.dart';
 Future<void> main() async {
   final client = RetryClient(http.Client());
   try {
-    print(await client.read('http://example.org'));
+    print(await client.read(Uri.http('example.org', '')));
   } finally {
     client.close();
   }

lib/http_retry.dart

@@ -7,7 +7,6 @@ import 'dart:math' as math;
 
 import 'package:async/async.dart';
 import 'package:http/http.dart';
-import 'package:pedantic/pedantic.dart';
 
 /// An HTTP client wrapper that automatically retries failing requests.
 class RetryClient extends BaseClient {
@@ -21,13 +20,13 @@ class RetryClient extends BaseClient {
   final bool Function(BaseResponse) _when;
 
   /// The callback that determines whether a request when an error is thrown.
-  final bool Function(dynamic, StackTrace) _whenError;
+  final bool Function(Object, StackTrace) _whenError;
 
   /// The callback that determines how long to wait before retrying a request.
   final Duration Function(int) _delay;
 
   /// The callback to call to indicate that a request is being retried.
-  final void Function(BaseRequest, BaseResponse, int) _onRetry;
+  final void Function(BaseRequest, BaseResponse?, int)? _onRetry;
 
   /// Creates a client wrapping [_inner] that retries HTTP requests.
   ///
@@ -50,17 +49,15 @@ class RetryClient extends BaseClient {
   /// error for which [whenError] returned `true`.
   RetryClient(
     this._inner, {
-    int retries,
-    bool Function(BaseResponse) when,
-    bool Function(Object, StackTrace) whenError,
-    Duration Function(int retryCount) delay,
-    void Function(BaseRequest, BaseResponse, int retryCount) onRetry,
-  })  : _retries = retries ?? 3,
-        _when = when ?? ((response) => response.statusCode == 503),
-        _whenError = whenError ?? ((_, __) => false),
-        _delay = delay ??
-            ((retryCount) =>
-                const Duration(milliseconds: 500) * math.pow(1.5, retryCount)),
+    int retries = 3,
+    bool Function(BaseResponse) when = _defaultWhen,
+    bool Function(Object, StackTrace) whenError = _defaultWhenError,
+    Duration Function(int retryCount) delay = _defaultDelay,
+    void Function(BaseRequest, BaseResponse?, int retryCount)? onRetry,
+  })  : _retries = retries,
+        _when = when,
+        _whenError = whenError,
+        _delay = delay,
         _onRetry = onRetry {
     RangeError.checkNotNegative(_retries, 'retries');
   }
@@ -74,9 +71,9 @@ class RetryClient extends BaseClient {
   RetryClient.withDelays(
     Client inner,
     Iterable<Duration> delays, {
-    bool Function(BaseResponse) when,
-    bool Function(Object, StackTrace) whenError,
-    void Function(BaseRequest, BaseResponse, int retryCount) onRetry,
+    bool Function(BaseResponse) when = _defaultWhen,
+    bool Function(Object, StackTrace) whenError = _defaultWhenError,
+    void Function(BaseRequest, BaseResponse?, int retryCount)? onRetry,
   }) : this._withDelays(
           inner,
           delays.toList(),
@@ -88,9 +85,9 @@ class RetryClient extends BaseClient {
   RetryClient._withDelays(
     Client inner,
     List<Duration> delays, {
-    bool Function(BaseResponse) when,
-    bool Function(Object, StackTrace) whenError,
-    void Function(BaseRequest, BaseResponse, int) onRetry,
+    required bool Function(BaseResponse) when,
+    required bool Function(Object, StackTrace) whenError,
+    required void Function(BaseRequest, BaseResponse?, int)? onRetry,
   }) : this(
           inner,
           retries: delays.length,
@@ -106,7 +103,7 @@ class RetryClient extends BaseClient {
 
     var i = 0;
     for (;;) {
-      StreamedResponse response;
+      StreamedResponse? response;
       try {
         response = await _inner.send(_copyRequest(request, splitter.split()));
       } catch (error, stackTrace) {
@@ -118,11 +115,11 @@ class RetryClient extends BaseClient {
 
         // Make sure the response stream is listened to so that we don't leave
         // dangling connections.
-        unawaited(response.stream.listen((_) {}).cancel()?.catchError((_) {}));
+        _unawaited(response.stream.listen((_) {}).cancel().catchError((_) {}));
       }
 
       await Future.delayed(_delay(i));
-      if (_onRetry != null) _onRetry(request, response, i);
+      _onRetry?.call(request, response, i);
       i++;
     }
   }
@@ -147,3 +144,12 @@ class RetryClient extends BaseClient {
   @override
   void close() => _inner.close();
 }
+
+bool _defaultWhen(BaseResponse response) => response.statusCode == 503;
+
+bool _defaultWhenError(Object error, StackTrace stackTrace) => false;
+
+Duration _defaultDelay(int retryCount) =>
+    const Duration(milliseconds: 500) * math.pow(1.5, retryCount);
+
+void _unawaited(Future<void>? f) {}

pubspec.yaml

@@ -1,18 +1,18 @@
 name: http_retry
-version: 0.1.2-dev
+version: 0.2.0-dev
 
 description: >-
   A wrapper for package:http clients that automatically retries requests
 homepage: https://github.com/dart-lang/http_retry
 
 environment:
-  sdk: '>=2.1.0 <3.0.0'
+  sdk: '>=2.12.0 <3.0.0'
 
 dependencies:
-  async: ^2.0.7
-  http: '>=0.11.0 <0.13.0'
-  pedantic: ^1.0.0
+  async: ^2.5.0
+  http: ^0.13.0
 
 dev_dependencies:
-  fake_async: ^1.0.0
-  test: ^1.2.0
+  fake_async: ^1.2.0
+  pedantic: ^1.10.0
+  test: ^1.16.0

test/http_retry_test.dart

@@ -13,23 +13,23 @@ void main() {
     test('a request has a non-503 error code', () async {
       final client = RetryClient(
           MockClient(expectAsync1((_) async => Response('', 502), count: 1)));
-      final response = await client.get('http://example.org');
+      final response = await client.get(Uri.http('example.org', ''));
       expect(response.statusCode, equals(502));
     });
 
     test("a request doesn't match when()", () async {
       final client = RetryClient(
           MockClient(expectAsync1((_) async => Response('', 503), count: 1)),
           when: (_) => false);
-      final response = await client.get('http://example.org');
+      final response = await client.get(Uri.http('example.org', ''));
       expect(response.statusCode, equals(503));
     });
 
     test('retries is 0', () async {
       final client = RetryClient(
           MockClient(expectAsync1((_) async => Response('', 503), count: 1)),
           retries: 0);
-      final response = await client.get('http://example.org');
+      final response = await client.get(Uri.http('example.org', ''));
       expect(response.statusCode, equals(503));
     });
   });
@@ -43,7 +43,7 @@ void main() {
         }, count: 2)),
         delay: (_) => Duration.zero);
 
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.statusCode, equals(200));
   });
 
@@ -58,7 +58,7 @@ void main() {
         when: (response) => response.headers['retry'] == 'true',
         delay: (_) => Duration.zero);
 
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.headers, containsPair('retry', 'false'));
     expect(response.statusCode, equals(503));
   });
@@ -75,7 +75,7 @@ void main() {
             error is StateError && error.message == 'oh no',
         delay: (_) => Duration.zero);
 
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.statusCode, equals(200));
   });
 
@@ -85,15 +85,15 @@ void main() {
         whenError: (error, _) => error == 'oh yeah',
         delay: (_) => Duration.zero);
 
-    expect(client.get('http://example.org'),
+    expect(client.get(Uri.http('example.org', '')),
         throwsA(isStateError.having((e) => e.message, 'message', 'oh no')));
   });
 
   test('retries three times by default', () async {
     final client = RetryClient(
         MockClient(expectAsync1((_) async => Response('', 503), count: 4)),
         delay: (_) => Duration.zero);
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.statusCode, equals(503));
   });
 
@@ -102,7 +102,7 @@ void main() {
         MockClient(expectAsync1((_) async => Response('', 503), count: 13)),
         retries: 12,
         delay: (_) => Duration.zero);
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.statusCode, equals(503));
   });
 
@@ -124,7 +124,7 @@ void main() {
         return Response('', 503);
       }, count: 4)));
 
-      expect(client.get('http://example.org'), completes);
+      expect(client.get(Uri.http('example.org', '')), completes);
       fake.elapse(const Duration(minutes: 10));
     });
   });
@@ -149,7 +149,7 @@ void main() {
           }, count: 4)),
           delay: (requestCount) => Duration(seconds: requestCount));
 
-      expect(client.get('http://example.org'), completes);
+      expect(client.get(Uri.http('example.org', '')), completes);
       fake.elapse(const Duration(minutes: 10));
     });
   });
@@ -178,7 +178,7 @@ void main() {
             Duration(seconds: 12)
           ]);
 
-      expect(client.get('http://example.org'), completes);
+      expect(client.get(Uri.http('example.org', '')), completes);
       fake.elapse(const Duration(minutes: 10));
     });
   });
@@ -190,12 +190,12 @@ void main() {
         retries: 2,
         delay: (_) => Duration.zero,
         onRetry: expectAsync3((request, response, retryCount) {
-          expect(request.url, equals(Uri.parse('http://example.org')));
-          expect(response.statusCode, equals(503));
+          expect(request.url, equals(Uri.http('example.org', '')));
+          expect(response?.statusCode, equals(503));
           expect(retryCount, equals(count));
           count++;
         }, count: 2));
-    final response = await client.get('http://example.org');
+    final response = await client.get(Uri.http('example.org', ''));
     expect(response.statusCode, equals(503));
   });