Skip to content

Rule proposal: Effect.tap with Metric.update re-implements Effect.trackSuccesses/trackErrors/trackDefects #658

Description

@mattiamanzati

Problem

Detects Effect.tap / Effect.tapError / Effect.tapDefect whose only job is to feed a value into a metric via Metric.update (or Metric.increment / Metric.incrementBy). This is the natural v3-era spelling for "record a metric on success/error/defect", and codebases migrating to v4 are full of it — but v4 ships dedicated combinators for exactly this: Effect.trackSuccesses, Effect.trackErrors, Effect.trackDefects (and the exit-level Effect.track). These are implemented as onExit + Metric.update internally, so behavior is identical; the win is intent and readability. Effect.trackSuccesses(sizeGauge) says "this metric observes this effect's successes" in one token, supports both the direct form (metric input is the channel value itself) and a mapper overload (trackSuccesses(metric, (value) => value.length)), and Metric.withConstantInput covers the "always update with 1" counter case. The tap spelling buries that intent inside a lambda and invites subtle drift, e.g. accidentally making the tap effectful in other ways.

Because the manual pattern compiles cleanly and behaves correctly, this is a pure prefer-this-API rule: its main value is discoverability of the new v4 track* combinators for users carrying v3 habits forward.

Bad — compiles cleanly, the rule should flag this

// RULE: manualMetricTapToTrack
// BAD: each tap* callback exists only to push a value into a metric.
// This is exactly what Effect.trackSuccesses / trackErrors / trackDefects
// do (they are implemented as onExit + Metric.update), stated in one token
// instead of a lambda.
import { Effect, Metric } from "effect"

const responseSize = Metric.gauge("response_size")
const requestErrors = Metric.counter("request_errors")
const requestDefects = Metric.counter("request_defects")

declare const handleRequest: Effect.Effect<number, Error>

const instrumented = handleRequest.pipe(
  Effect.tap((size) => Metric.update(responseSize, size)),
  Effect.tapError(() => Metric.update(requestErrors, 1)),
  Effect.tapDefect(() => Metric.update(requestDefects, 1))
)

Good

// RULE: manualMetricTapToTrack
// GOOD: the track* combinators name the intent directly — this metric
// observes this effect's successes / errors / defects. Same runtime
// behavior, no lambdas. Metric.withConstantInput covers the
// "always increment by 1" counter case.
import { Effect, Metric } from "effect"

const responseSize = Metric.gauge("response_size")
const requestErrors = Metric.withConstantInput(Metric.counter("request_errors"), 1)
const requestDefects = Metric.counter("request_defects")

declare const handleRequest: Effect.Effect<number, Error>

const instrumented = handleRequest.pipe(
  Effect.trackSuccesses(responseSize),
  Effect.trackErrors(requestErrors),
  Effect.trackDefects(requestDefects, () => 1)
)

Proposed rule behavior

  • Match calls resolving to Effect.tap, Effect.tapError, or Effect.tapDefect whose callback body is a single call resolving to Metric.update (or Metric.increment / Metric.incrementBy), verified via the checker that the first argument's type is a Metric. Map taptrackSuccesses, tapErrortrackErrors, tapDefecttrackDefects.
  • Also match Effect.tap's direct-effect overload — Effect.tap(Metric.update(counter, 1)) with no lambda at all — since that is the spelling real code uses (see the fixture hits below).
  • When the metric input is the callback parameter verbatim ((n) => Metric.update(gauge, n)), suggest the one-argument form Effect.trackSuccesses(gauge).
  • When the metric input is an expression of the parameter ((res) => Metric.update(gauge, res.length)), suggest the mapper overload Effect.trackSuccesses(gauge, (res) => res.length).
  • When the callback parameter is unused (constant input, e.g. () => Metric.update(counter, 1)), suggest either the mapper form with a constant or Metric.withConstantInput plus the one-argument form.
  • Do not fire when the callback body contains anything besides the single metric-update call (logging, other effects, Effect.all, etc.) — only the pure "tap exists solely to update one metric" shape is a safe rewrite.

Where this came up

Mined from a per-export sweep of the Effect module (v4): for each exported function, asking what manual pattern it replaces and whether that pattern is statically detectable; grounded against Effect-TS/effect and anomalyco/opencode; deduplicated against implemented tsgo diagnostics and prior rule-proposal issues.

Proposed rule name

manualMetricTapToTrack

Incremental true-positive recount: T3 Code

Reviewed pingdotgg/t3code at 01e05c15268d on 2026-09-14. Scope: tracked first-party TypeScript/JavaScript, including authored tests unless excluded by this proposal; vendored .repos, generated files, dependencies, build output and documentation examples excluded.

  • New T3 Code matches: 0. Counts refer to vetted diagnostic source sites, not observed production failures.
  • Previous reviewed count bucket: value:tp-5+.
  • Confirmed aggregate minimum: 5; label: value:tp-5+. The earlier range is preserved; the label uses its conservative lower bound plus these new sites, not an invented exact historical total.

Review notes. Metric updates occur inside generators or helper implementations, not a tap/tapError/tapDefect callback containing only one Metric update.

No new source location met the reviewed trigger and exclusions. Uncertain and version-inapplicable candidates were not added to the count.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    value:tp-5+Rule: 5+ vetted true positives in checked repositories

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions