From 254e3b00c7f41a54c588ec32564017f96f328252 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 15 Aug 2026 16:05:12 +0200 Subject: [PATCH] github_device_auth: retry transient network failures (v0.3.0) wake-alarm's copy carried a fix the other three lacked: `pollForToken` swallows a transient socket failure and keeps polling, because GitHub can close the connection at the exact moment the user authorizes and a phone can drop its network mid-poll. Without it, an approved grant is lost to a blip and the user has to start the whole flow again. Extracting only todo's version would have silently deleted that fix from wake-alarm. Taking it into the package instead gives it to all four apps. Catching `http.ClientException` is sufficient, and avoiding `dart:io` here is deliberate: diet-guard's desktop **web** build imports this package, and naming `SocketException` would break that compile. Checked rather than assumed -- a probe against a dead port shows package:http raises `_ClientSocketException`, which implements *both* `ClientException` and `SocketException`, so the single catch covers IO and web alike. 24 tests, 100% line coverage, clean analyze. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01U4Rj3s7K2qzrLNgJqqPhno --- github_device_auth/README.md | 17 ++++++++- github_device_auth/lib/src/device_auth.dart | 38 ++++++++++++++----- github_device_auth/pubspec.yaml | 2 +- github_device_auth/test/device_auth_test.dart | 18 +++++++++ 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/github_device_auth/README.md b/github_device_auth/README.md index c788bcb..be1b278 100644 --- a/github_device_auth/README.md +++ b/github_device_auth/README.md @@ -34,7 +34,7 @@ dependencies: github_device_auth: git: url: https://github.com/kuhyx/utils - ref: github_device_auth-v0.2.0 + ref: github_device_auth-v0.3.0 path: github_device_auth ``` @@ -57,6 +57,19 @@ them at all. diet-guard's desktop web build does exactly that, which is why this knob exists — it was in diet-guard's local copy and would have been lost had the package shipped only todo's simpler version. +## Transient-network retry + +`pollForToken` retries on `ClientException` rather than failing. GitHub can +close the connection at the exact moment the user authorizes, and a phone can +drop its network mid-poll — losing an approved grant to a socket blip is a real +bug wake-alarm had already hit and fixed locally. The other three copies had +not, so they inherit the fix here. + +`ClientException` alone is enough and `dart:io` is deliberately *not* imported: +this package is used by a web build, and `package:http` wraps a dead socket as +`_ClientSocketException`, which implements `ClientException` — verified, not +assumed. + ## One behaviour change from the extracted copies The dialog now catches `on Object`, not `on Exception`. @@ -72,7 +85,7 @@ pins it with a mock that throws an `Error` rather than an `Exception`. ```bash flutter pub get -flutter test # 23 tests +flutter test # 24 tests flutter test --coverage # 100% of lines flutter analyze # very_good_analysis, clean ``` diff --git a/github_device_auth/lib/src/device_auth.dart b/github_device_auth/lib/src/device_auth.dart index 5dc6ecb..afafe4b 100644 --- a/github_device_auth/lib/src/device_auth.dart +++ b/github_device_auth/lib/src/device_auth.dart @@ -135,15 +135,35 @@ class GitHubDeviceAuth { while (DateTime.now().isBefore(deadline)) { await _delay(Duration(seconds: intervalSeconds)); - final res = await _http.post( - Uri.parse(tokenUrl), - headers: const {'Accept': 'application/json'}, - body: { - 'client_id': clientId, - 'device_code': device.deviceCode, - 'grant_type': _grantType, - }, - ); + final http.Response res; + try { + res = await _http.post( + Uri.parse(tokenUrl), + headers: const {'Accept': 'application/json'}, + body: { + 'client_id': clientId, + 'device_code': device.deviceCode, + 'grant_type': _grantType, + }, + ); + } + // A transient network failure is not terminal: GitHub can close the + // connection at the very moment the user authorizes, and a phone can + // drop its network mid-poll. Keep polling until the device code + // actually expires -- otherwise an approved grant is lost to a socket + // blip. (wake-alarm had learned this the hard way; the other three + // copies had not.) + // + // `ClientException` alone is enough, and deliberately so: this package + // is imported by a **web** build (diet-guard's desktop wrapper), where + // importing `dart:io` to name `SocketException` would break + // compilation. Verified that no dart:io type escapes -- package:http + // wraps a dead socket as `_ClientSocketException`, which *is* a + // `ClientException` (it implements both), so this catch covers the IO + // case too. + on http.ClientException catch (_) { + continue; + } final json = jsonDecode(res.body) as Map; final token = json['access_token'] as String?; diff --git a/github_device_auth/pubspec.yaml b/github_device_auth/pubspec.yaml index 564bf50..cbd0a39 100644 --- a/github_device_auth/pubspec.yaml +++ b/github_device_auth/pubspec.yaml @@ -4,7 +4,7 @@ description: >- modal that drives it. Extracted from four apps that each carried a copy, two of them byte-identical. publish_to: 'none' -version: 0.2.0 +version: 0.3.0 environment: sdk: ^3.12.2 diff --git a/github_device_auth/test/device_auth_test.dart b/github_device_auth/test/device_auth_test.dart index dafd353..c4ea6cc 100644 --- a/github_device_auth/test/device_auth_test.dart +++ b/github_device_auth/test/device_auth_test.dart @@ -233,6 +233,24 @@ void main() { ); }); + test('retries a transient network failure instead of giving up', () async { + // GitHub can close the connection at the moment the user authorizes, + // and a phone can drop its network mid-poll. Losing an approved grant + // to a socket blip is the bug this retry exists to prevent. + var calls = 0; + final auth = authWith( + MockClient((_) async { + calls += 1; + if (calls == 1) { + throw http.ClientException('Connection closed before full header'); + } + return http.Response(jsonEncode({'access_token': 'tok'}), 200); + }), + ); + expect(await auth.pollForToken(device), 'tok'); + expect(calls, 2); + }); + test('gives up once the device code has expired', () async { const expired = DeviceCodeResponse( deviceCode: 'dc',