Skip to content

Surface Health Connect writeback failures instead of silent zero (#660) - #672

Merged
ryanbr merged 2 commits into
mainfrom
fix/hc-writeback-status-660
Jul 20, 2026
Merged

Surface Health Connect writeback failures instead of silent zero (#660)#672
ryanbr merged 2 commits into
mainfrom
fix/hc-writeback-status-660

Conversation

@ryanbr

@ryanbr ryanbr commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Fixes #660.

Problem

Health Connect writeback failures were swallowed. HealthConnectWriter.write() returned a bare Int, and each concern was wrapped in runCatching { … }.getOrDefault(0), so a revoked WRITE permission (or a provider error) collapsed to 0 — indistinguishable from "nothing to share." The background BLE-path write even logged a total failure as success (onSuccess-only). Net effect: a user could believe Health Connect sharing was active while every write silently failed. (Audited across four call sites, not just the two in the report.)

Fix

  • Structured result. write() / writeExercise() now return a WritebackResult(written, failures) where failures are PII-safe categories (PERMISSION_DENIED / REMOTE_ERROR) — never raw messages (the repo has a PiiRedactionTest; kept in that spirit). Coroutine CancellationException is rethrown, never mislabeled a failure.
  • Persisted, observable outcome. The writer records the last outcome to NoopPrefs, so even the background BLE-path write (which never touches the ViewModel) is visible. AppViewModel exposes it as a StateFlow and re-reads it when Data Sources opens.
  • UI. Under the writeback toggle, Data Sources now shows:
    • healthy → Last shared <ago> (calm/tertiary),
    • PERMISSION_DENIED → an accent line "Sharing paused — permission was revoked. Tap to re-grant." that relaunches the permission request,
    • transient → "Last share didn't finish — will retry."
  • Deliberately not auto-disabling the user's opt-in on a transient error (the report floated that) — a blip would thrash their setting and lose intent; a re-grant affordance is the safer surface.
  • Drift guard. WritebackResult.statusCode is pure and unit-tested, and pinned equal to the UI's NoopPrefs.HC_WB_* constants so the writer's persisted status and the Data Sources branches can't diverge.

Scope / parity

Android-only — Health Connect has no iOS equivalent (iOS writes to HealthKit), so no parity twin. If we want symmetry later, the iOS HealthKit writeback path is worth a parallel look, but it's out of scope for this HC-specific issue. New status strings localized de/es/fr/zh.

One intentional nuance: on a workout save, writeExercise runs then the periodic write() follows, so the persisted status reflects the last attempt (vitals). The 15-min vitals write is the dominant, always-running signal; exercise-only permission loss post-enable is a narrow case already re-prompted at enable time.

Verification

  • ./gradlew compileFullDebugKotlin
  • ./gradlew testFullDebugUnitTest --tests com.noop.ingest.WritebackResultTest --tests com.noop.ingest.HealthConnect* ✓ (new test pins the ok flag, the permission-outranks-generic mapping, and the UI-constant contract)
  • python3 Tools/i18n_audit.py ✓ (0 gaps, no new hardcoded literals)
  • ./gradlew lintVitalFullRelease -PstagingRelease

ryanbr added 2 commits July 20, 2026 16:08
Writeback failures were swallowed: HealthConnectWriter.write() returned a
bare Int, each concern wrapped in runCatching{}.getOrDefault(0), so a
revoked WRITE permission collapsed to 0 — indistinguishable from "nothing
to share" — and the BLE background path logged even a total failure as
success. A user could believe Health Connect sharing was active while
every write silently failed.

- write()/writeExercise now return a structured WritebackResult (records
  written + PII-safe failure categories: PERMISSION_DENIED / REMOTE_ERROR,
  never raw messages). Coroutine cancellation is rethrown, never mislabeled
  a failure.
- The writer persists the last outcome to NoopPrefs, so even the background
  BLE-path write is observable (it doesn't touch the ViewModel).
- Data Sources shows the outcome under the toggle: "Last shared <ago>" when
  healthy, an accent "permission revoked — tap to re-grant" (relaunches the
  permission request) on PERMISSION_DENIED, or a "didn't finish — will retry"
  line on a transient error. Deliberately NOT auto-disabling the user's
  opt-in on a transient failure.
- WritebackResult.statusCode is pure + unit-tested, and pinned equal to the
  UI's NoopPrefs.HC_WB_* constants so the writer and UI can't drift.

Android-only: Health Connect has no iOS equivalent (iOS uses HealthKit),
so no parity twin. New status strings localized de/es/fr/zh.

Verified: compileFullDebugKotlin, testFullDebugUnitTest (WritebackResultTest
+ HealthConnect*), i18n_audit (0 gaps), lintVitalFullRelease -PstagingRelease.

Author: ryanbr <mp3geek@gmail.com>
Re-review follow-up: in write(), getOrCreate() + the day read sat before
the per-concern runCatching blocks, so a provider race or DB error could
throw past recordStatus — leaving the Data Sources line a stale "OK" while
sharing was broken (the exact class this PR fixes). writeExercise already
guarded its getOrCreate; make write() consistent by wrapping the pre-insert
work so any non-cancellation throw records a categorized status and returns.

Author: ryanbr <mp3geek@gmail.com>
@ryanbr
ryanbr merged commit fbd5735 into main Jul 20, 2026
1 check passed
@ryanbr
ryanbr deleted the fix/hc-writeback-status-660 branch July 20, 2026 04:20
ryanbr added a commit that referenced this pull request Jul 20, 2026
Marketing version stays 9.0.3; Android versionCode 296->297 and iOS
CURRENT_PROJECT_VERSION 206->207 so the new staging build installs over the
previous one. Content since the last testing build: Health Connect writeback
failure surfacing (#672), SpO2/respiration empty-track explanation (#673),
WHOOP 5/MG raw-IMU offload persist (#675), hide stale sleep recompute entries
(#679), Oura activity MET research corpus + cap/rotation (#676/#680), trend
chart date alignment (#528), and corrupt-DB quarantine preservation (#681).
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
…nbr#660) (ryanbr#672)

write()/writeExercise return a structured WritebackResult (count + PII-safe failure categories) and persist the last outcome; Data Sources shows a failing share with a tap-to-re-grant action instead of a healthy-looking toggle. Android-only (Health Connect). Fixes ryanbr#660.
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
Marketing version stays 9.0.3; Android versionCode 296->297 and iOS
CURRENT_PROJECT_VERSION 206->207 so the new staging build installs over the
previous one. Content since the last testing build: Health Connect writeback
failure surfacing (ryanbr#672), SpO2/respiration empty-track explanation (ryanbr#673),
WHOOP 5/MG raw-IMU offload persist (ryanbr#675), hide stale sleep recompute entries
(ryanbr#679), Oura activity MET research corpus + cap/rotation (ryanbr#676/ryanbr#680), trend
chart date alignment (ryanbr#528), and corrupt-DB quarantine preservation (ryanbr#681).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reliability: surface Health Connect writeback failures instead of silently returning zero

1 participant