Centralize telemetry collector error handling via Poller.safe_invoke/3

[alco]

Summary

• Adds ElectricTelemetry.Poller.safe_invoke/3 and wraps every MFA built by periodic_measurements/2 in it, so transient collector failures no longer cause :telemetry_poller to permanently drop a measurement from its polling list.
• Silently absorbs :noproc/:timeout/:shutdown/:normal exits and ArgumentError (typical startup/restart races); logs a warning tagged with the MFA for anything else.
• Strips now-redundant defensive try/catch and with-fallthrough code from individual collectors in Electric.StackSupervisor.Telemetry, since safe_invoke/3 is the single contract for containing transient errors. The one ordering tweak worth noting: count_shapes/2 emits active_shapes before calling shape_counts/1, so a startup-race failure in the latter only drops total_shapes for the tick.

Background

:telemetry_poller catches MFA exceptions but returns error to its internal loop, which removes the measurement permanently. A single GenServer restart race or ETS-table startup race is enough to silently disable a metric for the lifetime of the poller. See issue electric-sql/alco-agent-tasks#32 for the full audit and design.

Note on semantics

User-supplied periodic measurement functions passed to ElectricTelemetry no longer have their exceptions propagated up to :telemetry_poller's own error logger — they are now caught and logged via ElectricTelemetry.Poller instead. This is called out in the changeset.

Test plan

• New ElectricTelemetry.PollerTest covers all catch clauses + the wrapping logic in periodic_measurements/2
• mix format clean
• mix compile clean for @core/sync-service and @core/electric-telemetry
• Electric.StackSupervisorTest passes
• Existing telemetry / stack-supervisor tests pass in CI

Refs electric-sql/alco-agent-tasks#32

[codecov]

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.59%. Comparing base (388ec63) to head (e3a0fb8).
⚠️ Report is 47 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...lectric-telemetry/lib/electric/telemetry/poller.ex	94.73%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4148       +/-   ##
===========================================
- Coverage   89.20%   66.59%   -22.62%     
===========================================
  Files          25      155      +130     
  Lines        2520    17676    +15156     
  Branches      636     4188     +3552     
===========================================
+ Hits         2248    11771     +9523     
- Misses        270     5901     +5631     
- Partials        2        4        +2     

Flag	Coverage Δ
electric-telemetry	71.40% <94.73%> (?)
elixir	71.40% <94.73%> (?)
packages/agents	47.33% <ø> (?)
packages/agents-runtime	81.42% <ø> (?)
packages/agents-server	65.69% <ø> (?)
packages/agents-server-ui	0.00% <ø> (?)
packages/electric-ax	31.19% <ø> (?)
packages/experimental	87.73% <ø> (ø)
packages/react-hooks	86.48% <ø> (ø)
packages/start	82.83% <ø> (ø)
packages/typescript-client	94.30% <ø> (ø)
packages/y-electric	56.05% <ø> (ø)
typescript	66.43% <ø> (-22.78%)	⬇️
unit-tests	66.59% <94.73%> (-22.62%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13b3967b18

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude Code Review

Summary

The new commit (e3a0fb8bb) drops the local try/catch from report_retained_wal_size/3 and the defensive with-fallthrough from count_shapes/2, correctly delegating all error handling to safe_invoke. One remaining issue from the previous iterations — the :builtin measurements path — is still not wrapped.

What's Working Well

• Removing the catch-all try/catch from report_retained_wal_size/3 and delegating to safe_invoke correctly restores warning-level logging for unexpected errors. This is exactly the right fix for the observability regression raised in the last review.
• Removing the "do not raise if stack down" test is intentional and correct — the functions are now expected to raise when called without safe_invoke, and the poller wrapper is the intended contract.
• The ordering of count_shapes/2 (emit active_shapes before the potentially-failing shape_counts/1 call) is clean and well-commented.
• safe_invoke/3 and its test suite remain solid.

Issues Found

Important (Should Fix)

:builtin measurements still not wrapped in safe_invoke

File: packages/electric-telemetry/lib/electric/telemetry/poller.ex:30-31

:builtin ->
  module.builtin_periodic_measurements(telemetry_opts)

The 2-arity periodic_measurements/2 fallback (line 49-50) was correctly fixed to pipe through &wrap_mfa/1, but the :builtin branch inside flat_map still returns MFAs from builtin_periodic_measurements/1 without going through wrap_mfa/1. Any crash in a builtin measurement will still be surfaced directly to :telemetry_poller, which will permanently remove it from the polling list — the exact problem this PR was designed to fix.

The fix is a one-liner (same suggestion as previous two iterations):

:builtin ->
  module.builtin_periodic_measurements(telemetry_opts)
  |> Enum.map(&wrap_mfa/1)

Suggestions (Nice to Have)

Silent ArgumentError swallowing is unexplained — packages/electric-telemetry/lib/electric/telemetry/poller.ex:73-74. The rescue ArgumentError -> :ok clause silently absorbs the error with no log output. A one-line comment (# ETS tables not yet created during startup/restart) would make this intent clear to the next reader. Carried over from previous two iterations.

---

Issue Conformance

No linked public issue (refs alco-agent-tasks#32). Remains a non-blocking concern.

---

Previous Review Status

Issue	Status
Critical: Failing test for cold-stack measurements	Fixed in iteration 2
Important: :builtin measurements not wrapped	Still not addressed
Important: Observability regression in report_retained_wal_size/3	Fixed — try/catch removed, safe_invoke handles at warning level
Suggestion: ArgumentError comment	Not addressed

---

Review iteration: 3 | 2026-04-27

[netlify]

✅ Deploy Preview for electric-next ready!

Name	Link
🔨 Latest commit	e3a0fb8
🔍 Latest deploy log	https://app.netlify.com/projects/electric-next/deploys/69efebf8efa27f0008975899
😎 Deploy Preview	https://deploy-preview-4148--electric-next.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3a0fb8bb9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".