Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve Span on Interruption #3578

Draft
wants to merge 15 commits into
base: next-minor
Choose a base branch
from

Conversation

mikearnaldi
Copy link
Member

@mikearnaldi mikearnaldi commented Sep 9, 2024

The following code:

import { Effect, Fiber } from "effect"

const program = Effect.gen(function*() {
  const fiber = yield* Effect.fork(
    Effect.uninterruptibleMask((restore) =>
      restore(
        Effect.sleep("1 second").pipe(
          Effect.withSpan("sleep")
        )
      ).pipe(
        Effect.tapErrorCause((cause) => Effect.logError("Got cause:", cause))
      )
    )
  )

  yield* Effect.sleep("250 millis")
  yield* Fiber.interrupt(fiber)
  yield* Fiber.await(fiber)
  yield* Effect.logInfo("Done")
})

Effect.runFork(program)

now prints:

michaelarnaldi@Michaels-MacBook-Pro effect % pnpm tsx scratchpad/interrupt-span.ts 
timestamp=2024-09-09T15:43:01.173Z level=ERROR fiber=#1 message="Got cause:" cause="InterruptedException: Fiber Interrupted by: #0
    at sleep (/Users/michaelarnaldi/repositories/effect/scratchpad/interrupt-span.ts:8:18)"
timestamp=2024-09-09T15:43:01.175Z level=INFO fiber=#0 message=Done
michaelarnaldi@Michaels-MacBook-Pro effect % 

I am not too sure this is a wanted & helpful feature, up for discussion

Copy link

changeset-bot bot commented Sep 9, 2024

🦋 Changeset detected

Latest commit: f400eaa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 31 packages
Name Type
effect Minor
@effect/cli Major
@effect/cluster-browser Major
@effect/cluster-node Major
@effect/cluster-workflow Major
@effect/cluster Major
@effect/experimental Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node-shared Major
@effect/platform-node Major
@effect/platform Major
@effect/printer-ansi Major
@effect/printer Major
@effect/rpc-http Major
@effect/rpc Major
@effect/schema Major
@effect/sql-d1 Major
@effect/sql-drizzle Major
@effect/sql-kysely Major
@effect/sql-mssql Major
@effect/sql-mysql2 Major
@effect/sql-pg Major
@effect/sql-sqlite-bun Major
@effect/sql-sqlite-node Major
@effect/sql-sqlite-react-native Major
@effect/sql-sqlite-wasm Major
@effect/sql Major
@effect/typeclass Major
@effect/vitest Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tim-smart
Copy link
Member

I think it is worthwhile, debugging interrupts is painful at the moment.

@tim-smart tim-smart force-pushed the feat/preserve-span-on-interruption branch from 1841b80 to bf02ecb Compare September 10, 2024 04:26
@mikearnaldi
Copy link
Member Author

I think it is worthwhile, debugging interrupts is painful at the moment.

it is but not sure how this helps debugging interrupts, it tells you where the program stopped and not who stopped it

@github-actions github-actions bot force-pushed the next-minor branch 2 times, most recently from 8ac627f to aa5f3eb Compare September 10, 2024 07:43
@mikearnaldi
Copy link
Member Author

/rebase

@tim-smart
Copy link
Member

/rebase

@tim-smart
Copy link
Member

it is but not sure how this helps debugging interrupts, it tells you where the program stopped and not who stopped it

I feel like we could capture a span where .interrupt is called.

@tim-smart
Copy link
Member

It seems that is what you did in the latest commit? We could have the origin span and the receiver span, and then maybe set the error cause to the origin if present.

@mikearnaldi
Copy link
Member Author

It seems that is what you did in the latest commit? We could have the origin span and the receiver span, and then maybe set the error cause to the origin if present.

No what I did is capture where Effect.interruptible is called not where Fiber.interrupt is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Discussion Ongoing
Development

Successfully merging this pull request may close these issues.

6 participants