Enforce cancellation-safe runCatching: runCatchingCancellable + SwallowedCancellation lint check

[bmander]

Makes the "runCatching swallows CancellationException in coroutine code" bug class — audited in #1928, first fixed in #1921 — impossible to reintroduce, deterministically, in CI. The "easy to get wrong" pattern becomes a build failure with a one-line fix.

Two parts

1. runCatchingCancellable — the sanctioned wrapper

org.onebusaway.android.util. Behaviour-identical to kotlin.runCatching but rethrows CancellationException, so a cancelled coroutine propagates instead of being captured into Result.failure. inline (mirrors runCatching), so block may call suspend functions and the guard costs nothing.

inline fun <T> runCatchingCancellable(block: () -> T): Result<T> =
    runCatching(block).onFailure { if (it is CancellationException) throw it }

2. SwallowedCancellation lint check (:lint-rules)

Fails the build on a bare runCatching whose nearest enclosing named function is suspend — matching detekt's SuspendFunSwallowedCancellation, but in the module's existing custom-lint infrastructure (no new tool, same strict -PwarningsAsErrors gate, no baseline). Suspend is detected via the Continuation parameter on the light method.

• Lambdas are transparent, so async { runCatching { … } } inside a suspend function is caught.
• A runCatching in a coroutine-builder lambda inside a non-suspend function is deliberately out of scope (boundary not visible without heuristics) — e.g. RouteMapController's launch-lambda sites keep their correct inline guards; the Compose sheet-animation runCatchings in HomeScreen sit in @Composables and are correctly ignored.
• With one sanctioned wrapper the rule is a symbol ban — no fragile "does this lambda suspend" analysis, no guard-detection. A runCatching around purely non-suspending work in a suspend function is still flagged; the wrapper is a strict superset, so it's never wrong.

Migration

Every flagged site moved onto the wrapper — the api/repository sites from the #1928 audit plus four pure-but-in-suspend-fn sites the rule newly surfaced (GeocodeRepository, TripResultsRepository ×2, TripPlanRepository.plan), and StopsForRouteRepository.routeMap (from #1921, which merged meanwhile — the safeguard immediately caught it, exactly as intended). Net reduction in lines: the wrapper reads cleaner than the inline guards. BikeStationsRepository keeps its inner catch (CancellationException) { throw e } (the check covers runCatching, not try/catch — for those, add a leading cancellation catch by hand).

Also renamed TimeDomainIssueRegistry → OneBusAwayIssueRegistry (no longer time-only) + manifest attribute; added detector unit tests, a lint-rules/README section, and a CLAUDE.md section.

Validation

• :lint-rules:test — green (6 new detector tests: fires in a suspend fn, fires through a lambda, ignores the wrapper, ignores non-suspend fns and a same-named non-kotlin runCatching).
• App compiles clean under -PwarningsAsErrors; unit suite green.
• A full app lint run flagged exactly one SwallowedCancellation — a temporary probe (since removed) — and zero real sites, confirming in one pass: registry discovery, the rule firing, and complete migration. (It also surfaced 2 pre-existing RestrictedApi errors on Hct from Add adjacency-aware stop and route focus to the map #1865 and 2 ModifierParameter warnings in untouched files — a known local-vs-CI lint discrepancy, unrelated to this change.)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved coroutine cancellation handling across network, search, trip-planning, weather, and data-loading flows.
  • Prevented cancelled operations from being incorrectly reported as ordinary failures, improving structured concurrency and cancellation propagation.

• New Features

  • Added automated checks to flag unsafe cancellation handling in coroutine code.

• Documentation

  • Added guidance for safely handling cancellation and using the cancellation-aware error-handling approach.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds runCatchingCancellable, migrates coroutine result handling across Android repositories, and introduces the SwallowedCancellation Android Lint check with registry integration, tests, documentation, and enforcement updates.

Changes

Cancellation-safe coroutine handling

Layer / File(s)	Summary
Shared wrapper and repository migration onebusaway-android/src/main/java/org/onebusaway/android/util/RunCatchingCancellable.kt, onebusaway-android/src/main/java/org/onebusaway/android/{api,data,map,ui}/...	Adds runCatchingCancellable, which rethrows CancellationException while returning other failures as Result.failure, and replaces manual cancellation handling across coroutine call sites.
Lint detection and registration lint-rules/build.gradle.kts, lint-rules/src/main/java/org/onebusaway/lint/..., lint-rules/src/test/java/org/onebusaway/lint/SwallowedCancellationDetectorTest.kt	Adds and registers SwallowedCancellation, detects runCatching in suspend functions, updates lint registry discovery, and tests matching and non-matching cases.
Guidance and enforcement documentation CLAUDE.md, lint-rules/README.md	Documents cancellation-safe coroutine patterns, detector scope, lifecycle, enforcement status, and the zero-violation baseline.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SuspendRepository
  participant runCatchingCancellable
  participant Coroutine
  SuspendRepository->>runCatchingCancellable: Execute coroutine operation
  runCatchingCancellable->>Coroutine: Rethrow CancellationException
  runCatchingCancellable-->>SuspendRepository: Return Result for other failures

Loading

Possibly related PRs

• OneBusAway/onebusaway-android#1921: Introduces and unifies the StopsForRouteRepository.routeMap logic that this change updates to use cancellation-safe result handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 34.48% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: a cancellation-safe runCatching helper plus a lint check to prevent swallowed cancellation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/lint-swallowed-cancellation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

lint-rules/src/main/java/org/onebusaway/lint/SwallowedCancellationDetector.kt (1)

52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider covering mapCatching and recoverCatching in the future.

While mapCatching and recoverCatching are typically used for synchronous mappings, they internally use runCatching and execute inline blocks. If a developer invokes a suspend function inside them, they will also silently swallow CancellationException.

Since there are no cancellable wrappers for these operators yet, this is purely a future-proofing thought: you might consider eventually expanding the detector's scope and creating corresponding wrappers (e.g., mapCatchingCancellable) to make the protection completely foolproof.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@lint-rules/src/main/java/org/onebusaway/lint/SwallowedCancellationDetector.kt`
at line 52, Keep the current SwallowedCancellationDetector scope unchanged for
now; record mapCatching and recoverCatching as future coverage and wrapper
candidates without modifying getApplicableMethodNames or adding wrappers in this
change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@lint-rules/src/main/java/org/onebusaway/lint/SwallowedCancellationDetector.kt`:
- Line 52: Keep the current SwallowedCancellationDetector scope unchanged for
now; record mapCatching and recoverCatching as future coverage and wrapper
candidates without modifying getApplicableMethodNames or adding wrappers in this
change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cdfdd427-3d1e-4f9e-8a7f-76bf22c8d064

📥 Commits

Reviewing files that changed from the base of the PR and between 4e3dcec and 70c0060.

📒 Files selected for processing (21)

• CLAUDE.md
• lint-rules/README.md
• lint-rules/build.gradle.kts
• lint-rules/src/main/java/org/onebusaway/lint/OneBusAwayIssueRegistry.kt
• lint-rules/src/main/java/org/onebusaway/lint/SwallowedCancellationDetector.kt
• lint-rules/src/test/java/org/onebusaway/lint/SwallowedCancellationDetectorTest.kt
• onebusaway-android/src/main/java/org/onebusaway/android/api/data/StopsForRouteRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/api/data/SurveyDataSource.kt
• onebusaway-android/src/main/java/org/onebusaway/android/api/net/ObaApiProvider.kt
• onebusaway-android/src/main/java/org/onebusaway/android/map/bike/BikeStationsRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/home/weather/WeatherRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/mylists/MyListRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/routeinfo/RouteInfoRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/search/RouteSearchRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/search/StopSearchRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/searchresults/SearchResultsRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/tripinfo/TripInfoRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/tripplan/GeocodeRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/tripplan/TripPlanRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/tripresults/TripResultsRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/util/RunCatchingCancellable.kt