Skip to content

Rule proposal: Promise-returning thunk in Effect.try leaves the rejection outside the error channel #657

Description

@mattiamanzati

Problem

Detects calls to Effect.try (both the bare-thunk form and the { try, catch } options form) whose thunk returns a Promise — including async arrows. Effect.try is the constructor for synchronous computations that may throw: it runs the thunk, catches anything thrown synchronously, and puts the return value straight into the success channel. When the thunk returns a Promise, nothing is awaited: the produced type is Effect<Promise<A>, UnknownError>, the Promise is already running by the time the effect succeeds, and — worse — its eventual rejection never reaches the catch mapper or the error channel at all. fetch(url) almost never throws synchronously, so the carefully written catch handler is dead code and the real failure surfaces as an unhandled promise rejection outside Effect entirely. The fix is Effect.tryPromise, which awaits the Promise, routes rejections through the same catch: (error) => E mapper, and even hands the thunk an AbortSignal wired to fiber interruption — the migration keeps the catch mapper unchanged.

This is the direct sibling of the already-implemented lazyPromiseInEffectSync diagnostic, which covers the identical footgun on Effect.sync; Effect.try is currently uncovered. Related: #405 (Promise silently wrapped in a success channel, the Effect.sync/Effect.succeed side), #475 (already-started Promise passed to Effect.promise/tryPromise).

Why the compiler is silent / what breaks at runtime: Effect.try's type parameter A is unconstrained, so A = Promise<Response> infers cleanly and the program type-checks. At runtime the effect "succeeds" immediately with a pending Promise; a downstream Effect.catchAll/catchTag never fires for the request's failure, and a network error becomes an unhandled rejection that crashes or warns outside the fiber's error model.

Bad — compiles cleanly, the rule should flag this

// RULE: effectTryReturnsPromise
// BAD: Effect.try is for synchronous computations. fetch() returns a Promise,
// so the success channel holds a raw, unawaited Promise<Response> — and since
// fetch almost never throws synchronously, the catch mapper below is dead
// code: a network failure bypasses E entirely and escapes as an unhandled
// promise rejection.
import { Data, Effect } from "effect"

class RequestError extends Data.TaggedError("RequestError")<{
  readonly cause: unknown
}> {}

//      ┌─── Effect<Promise<Response>, RequestError>
//      ▼
const fetchUser = (id: string) =>
  Effect.try({
    try: () => fetch(`https://api.example.com/users/${id}`),
    catch: (cause) => new RequestError({ cause })
  })

// async thunk — same footgun with the bare-thunk form:
// Effect<Promise<string>, UnknownError>, rejection never lands in E.
const readBody = (res: Response) => Effect.try(async () => res.text())

Good

// RULE: effectTryReturnsPromise
// GOOD: Effect.tryPromise awaits the Promise and routes its rejection through
// the same catch mapper into the typed error channel — the migration keeps
// catch unchanged. It also passes an AbortSignal tied to fiber interruption.
import { Data, Effect } from "effect"

class RequestError extends Data.TaggedError("RequestError")<{
  readonly cause: unknown
}> {}

//      ┌─── Effect<Response, RequestError>
//      ▼
const fetchUser = (id: string) =>
  Effect.tryPromise({
    try: (signal) => fetch(`https://api.example.com/users/${id}`, { signal }),
    catch: (cause) => new RequestError({ cause })
  })

//      ┌─── Effect<string, UnknownError>
//      ▼
const readBody = (res: Response) => Effect.tryPromise(() => res.text())

Proposed rule behavior

  • On a call expression resolving to Effect.try (module-level resolution via the checker, not identifier text), locate the thunk: the sole argument in the bare form, or the try property in the { try, catch } options form.
  • Report when the thunk is an async function/arrow, or when the checker type of the thunk's return value is Promise/PromiseLike (its instantiated A has a callable then matching the thenable shape).
  • Message: the success channel holds an unawaited Promise whose rejection escapes both the catch mapper and the error channel; suggest Effect.tryPromise, noting the catch mapper carries over unchanged.
  • Offer a codefix rewriting Effect.try to Effect.tryPromise (drop the async keyword on the thunk when its body is a single awaitless expression is optional polish; the identifier swap alone is already correct).
  • Do not report when A is a union where only some members are thenables, or when the thunk's return type is generic/unresolved at the call site — only fire on a definite Promise/PromiseLike to keep the rule zero-false-positive, mirroring lazyPromiseInEffectSync.
  • Both reference codebases wrap only synchronous work in Effect.try (JSON.parse, decoder calls, WebSocket construction, process.kill, image decode), so no exclusions beyond the definite-thenable requirement appear necessary.

Where this came up

No true-positive occurrences found in Effect-TS/effect@c3c7647 or anomalyco/opencode@550d1ff — proposed from the API sweep; both reference codebases are expert-written, so absence there is weak negative signal.

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

effectTryReturnsPromise

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions