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

Keep stack traces in withError #3573

Closed
wants to merge 2 commits into from
Closed

Conversation

serras
Copy link
Member

@serras serras commented Jan 26, 2025

This is an attempt to fix #3351. The idea is to "inline" the definition of withError, and within it carefully copy stack traces. Since the "inner exception" already contains the stack trace including any withError, we just need to copy it, not change it in any way.

@serras serras requested review from nomisRev and kyay10 January 26, 2025 17:17
@serras serras self-assigned this Jan 26, 2025
callsInPlace(transform, AT_MOST_ONCE)
}
// recover({ return block(this) }) { raise(transform(it)) }
val raise = DefaultRaise(false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean that innerException would always have no stack trace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would that be the case? As I understand the inner stack trace comes from the point where the RaiseCancellationException is first thrown.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the inner exception is always going to be a NoTrace because it's created by a DefaultRaise(false). Otherwise, we would be creating a stack trace every time, which would defeat our optimizations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is definitely true!

@serras
Copy link
Member Author

serras commented Jan 26, 2025

It was too good to be true...

@serras serras closed this Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

["Request"] better stack traces when tracing withError
2 participants