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

add Cause.Annotated #3594

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

add Cause.Annotated #3594

wants to merge 22 commits into from

Conversation

tim-smart
Copy link
Member

No description provided.

Copy link

changeset-bot bot commented Sep 12, 2024

🦋 Changeset detected

Latest commit: 9b4e760

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 Author

@mikearnaldi This was an idea for a more flexible annotation system for Cause. Maybe something for 4.0?

@mikearnaldi
Copy link
Member

@mikearnaldi This was an idea for a more flexible annotation system for Cause. Maybe something for 4.0?

We used to have annotations but we removed them in favor of carrying the span in a proxy due to re-fail losing the span

@tim-smart
Copy link
Member Author

We used to have annotations but we removed them in favor of carrying the span in a proxy due to re-fail losing the span

This goes the route of a re-fail rehydrating the annotations.

@mikearnaldi
Copy link
Member

Will need to check the details but as far as I understand it's not really possible, spans are linked to a specific error, re-failing with a different error should not hydrate spans

@tim-smart
Copy link
Member Author

Will need to check the details but as far as I understand it's not really possible, spans are linked to a specific error, re-failing with a different error should not hydrate spans

When you annotate a Die or Fail, it will also add the annotations to the error object using a Proxy (like we do now). If during a re-fail it finds existing annotations, then it will re-add them to the cause.

@github-actions github-actions bot force-pushed the next-minor branch 2 times, most recently from 387aa39 to b465b4a Compare October 1, 2024 14:16
@mikearnaldi
Copy link
Member

Will need to check the details but as far as I understand it's not really possible, spans are linked to a specific error, re-failing with a different error should not hydrate spans

When you annotate a Die or Fail, it will also add the annotations to the error object using a Proxy (like we do now). If during a re-fail it finds existing annotations, then it will re-add them to the cause.

if it re-checks then we might even be able to not annotate / proxy, we just check if the failure is === to the previous and we preserve annotations

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.

7 participants