Skip to content

Commit d93f52d

Browse files
authored
fix(ci): unbreak Dart Format and Dart Analyze on main (#1181)
`Build` has been failing on `main` and therefore on every PR, regardless of what the PR changes — visible on #1180, `max/fix-ios-audio-session-before-recording` and `sync-upstream-2.11.0`, all of which show `Build ✗` with `Changeset Check ✓`. Two jobs are responsible. ## Dart Format Three files under `scripts/` are not formatted, so `dart format . --set-exit-if-changed` exits 1. Straight from the CI log: ``` Formatted scripts/check_version.dart Formatted scripts/create_change.dart Formatted scripts/create_version.dart Formatted 227 files (3 changed) in 0.72 seconds. ##[error]Process completed with exit code 1 ``` Formatting only, no behaviour change. ## Dart Analyze ``` lib/src/token_source/caching.dart:138:9 • unawaited_return_in_try_block lib/src/token_source/caching.dart:144:7 • unawaited_return_in_try_block ``` `CachingTokenSource.fetch` returned `resultFuture` from inside its `try`, so the surrounding `catch` could not observe a failure from that future and `finally` removed the in-flight entry before it settled. Both returns now `await`. Behaviour is unchanged — the completer is completed on the line immediately before each return, so the awaited future is already resolved — but errors now route through the existing `catch`, and the in-flight map is cleared only once the request settles. `AGENTS.md` calls out un-awaited async state updates as a recurring bug class here, so this is a real latent issue rather than a lint silence. Includes a `patch type="fixed"` changeset. ## The analysis_options churn, and a coverage hole it was hiding `flutter analyze` kept leaving `analysis_options.yaml` modified. The cause is `AnalysisOptionsMigration` (`flutter_tools/lib/src/migrations/analysis_options_migration.dart`, invoked from `project.dart:429`), which appends seven patterns — `build/**`, `android/**`, `ios/**`, `web/**`, `windows/**`, `macos/**`, `linux/**` — on every `flutter analyze` / `pub get` / `run`. It is unconditional; there is no feature flag or config to disable it, unlike the UIScene migrator's `enable-uiscene-migration`. Committing the patterns makes it a no-op. But one of them matters: **this package keeps real Dart source in `web/`** (the E2EE worker), not just assets, so `web/**` silently drops it from analysis. That was already happening in CI, since `flutter analyze` runs the migrator before it analyzes. Passing the path explicitly does **not** work — `dart analyze web/` still honours the root exclude. I confirmed this by appending a deliberate type error to `web/e2ee.logger.dart`: it reported `No issues found!`. Giving `web/` its own `analysis_options.yaml` creates a separate context that is genuinely analyzed; the same canary then correctly reports `return_of_invalid_type`. That file mirrors the root's error overrides and its formatter settings (`page_width: 120`, `trailing_commas: preserve`) so `dart format` output is unaffected, and `build.yaml` gains a `dart analyze web/` step. ## Verified locally ``` dart format . --set-exit-if-changed PASS (227 files, 0 changed) flutter analyze PASS (No issues found) dart analyze web/ PASS (and fails on an injected error) flutter test PASS (+398) dart run scripts/check_version.dart PASS import_sorter --exit-if-changed PASS ``` ## Not included Pinning the Flutter version in `.github/actions/setup-flutter/action.yml`, which currently passes only `channel: stable` and so tracks whatever the newest stable release is. That is a genuine fragility — `unawaited_return_in_try_block` arrived this way — but it is a separate decision from unbreaking CI, and worth its own discussion. Draft because #1180 should confirm this actually turns `Build` green once rebased on it.
1 parent f62d479 commit d93f52d

8 files changed

Lines changed: 69 additions & 2 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
patch type="fixed" "CachingTokenSource.fetch now awaits its result, so errors surface and the in-flight entry is cleared correctly"

.github/workflows/build.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ jobs:
6666
- uses: ./.github/actions/setup-flutter
6767
- name: Dart Analyze Check
6868
run: flutter analyze
69+
# `flutter analyze` runs Flutter's AnalysisOptionsMigration first, which
70+
# forces `web/**` into analysis_options.yaml. This package keeps real Dart
71+
# source there (the E2EE worker), so analyze it separately -- `dart
72+
# analyze` does not run migrators.
73+
- name: Dart Analyze Check (web sources)
74+
run: dart analyze web/
6975

7076
dart-test-check:
7177
name: Dart Test

analysis_options.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@ analyzer:
3333
# Manager is enabled and this package ships Package.swift.
3434
- build/**
3535
- example/build/**
36+
# Flutter's AnalysisOptionsMigration (flutter_tools) appends these seven
37+
# patterns to this file on every `flutter analyze`/`pub get`/`run`. There is
38+
# no opt-out, so they are committed here to stop the file being rewritten.
39+
# Note this excludes web/, which in this package holds real Dart source (the
40+
# E2EE worker) rather than just assets -- CI analyzes it explicitly with
41+
# `dart analyze web/`, which does not run migrators.
42+
- android/**
43+
- ios/**
44+
- web/**
45+
- windows/**
46+
- macos/**
47+
- linux/**
3648

3749
linter:
3850
rules:

lib/src/token_source/caching.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,13 @@ class CachingTokenSource implements TokenSourceConfigurable {
135135
final cached = await _store.retrieve();
136136
if (cached != null && cached.options == options && _validator(cached.options, cached.response)) {
137137
completer.complete(cached.response);
138-
return resultFuture;
138+
return await resultFuture;
139139
}
140140

141141
final response = await _wrapped.fetch(options);
142142
await _store.store(options, response);
143143
completer.complete(response);
144-
return resultFuture;
144+
return await resultFuture;
145145
} catch (e, stackTrace) {
146146
completer.completeError(e, stackTrace);
147147
rethrow;

scripts/check_version.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#!/usr/bin/env dart
2+
23
/*
34
* Copyright 2025 LiveKit
45
*

scripts/create_change.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#!/usr/bin/env dart
2+
23
/*
34
* Copyright 2025 LiveKit
45
*

scripts/create_version.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#!/usr/bin/env dart
2+
23
/*
34
* Copyright 2025 LiveKit
45
*

web/analysis_options.yaml

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#
2+
# Copyright 2025 LiveKit
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
# The repository-root analysis_options.yaml has to exclude `web/**`: Flutter's
17+
# AnalysisOptionsMigration appends that pattern on every `flutter analyze` and
18+
# offers no way to opt out. These files are real Dart source (the E2EE worker),
19+
# not web assets, so this file gives them their own analysis context and keeps
20+
# them covered. CI analyzes them via `dart analyze web/`.
21+
22+
include: package:lints/recommended.yaml
23+
24+
analyzer:
25+
errors:
26+
constant_identifier_names: ignore
27+
use_super_parameters: ignore
28+
avoid_print: ignore
29+
deprecated_member_use_from_same_package: ignore
30+
31+
linter:
32+
rules:
33+
# Preference of the SDK
34+
prefer_single_quotes: true
35+
prefer_final_locals: true
36+
unnecessary_brace_in_string_interps: false
37+
avoid_print: true
38+
39+
# Enforce this for correct async logic
40+
unawaited_futures: true
41+
discarded_futures: true
42+
43+
formatter:
44+
page_width: 120
45+
trailing_commas: preserve

0 commit comments

Comments
 (0)