8000 adds support for logging failures with traces by somdoron · Pull Request #9738 · zio/zio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

adds support for logging failures with traces #9738

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: series/2.x
Choose a base branch
from
Open

adds support for logging failures with traces #9738

wants to merge 4 commits into from
+150 −0

Conversation

somdoron
Copy link
Contributor
@somdoron somdoron commented Apr 1, 2025

An alternative to #9488, fixes #9485.

ZIO 2.0 removed zio-logging's logThrowable; in ZIO 2.0, only Causes can be logged. This PR restores the ability to log failures that are not wrapped in causes.

catchAllCause does not give access to the original error, and it is also dangerous because it catches defects as well.

Now users can use catchAll or catchAllTrace to log the error safely:

effect.catchAll(e => ZIO.logErrorFailure(e))
effect.catchAllTrace {
  case (e, trace) => ZIO.logErrorFailure(e, trace)
}

tapError and tapErrorTrace can also be used:

effect.tapError(e => ZIO.logErrorFailure(e))
effect.tapErrorTrace {
  case (e, trace) => ZIO.logErrorFailure(e, trace)
}

cc @quelgar

@somdoron somdoron changed the title fadds support for logging failures with traces adds support for logging failures with traces Apr 1, 2025
hearnadam
hearnadam reviewed Apr 2, 2025
View reviewed changes
@somdoron somdoron requested a review from hearnadam April 9, 2025 12:23
hearnadam
hearnadam reviewed Apr 9, 2025
View reviewed changes
core/shared/src/main/scala/zio/ZIO.scala
* Logs the specified failure at the current log level.
*/
def logFailure(e: => Any)(implicit trace: Trace): UIO[Unit] =
ZIO.logCause(Cause.fail(e))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am fine with this change overall, but I wonder if users would expect this to capture a trace given it's not an explicit parameter. @kyri-petrou what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to have trace with default Empty, however scala only allow one overload with default params. So I had to split it into two overloads, one with error only and one with error and trace.

In total there are 3 overloads, one with error only, one with error and trace and one with message, error and trace with default parameter.

I'm fine with removing the first one with error only if it make it clearer.

@somdoron
Copy link
Contributor Author

@kyri-petrou can you please take a look?

@kyri-petrou kyri-petrou mentioned this pull request May 20, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hearnadam hearnadam hearnadam left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No way to catch or tap Error as Cause without defects
2 participants
@somdoron @hearnadam
0