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

Automatic error logging #2664

Open
decoursin opened this issue Apr 30, 2024 · 10 comments
Open

Automatic error logging #2664

decoursin opened this issue Apr 30, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@decoursin
Copy link

decoursin commented Apr 30, 2024

What is the problem this feature would solve?

More convenient logging for errors.

What is the feature you are proposing to solve the problem?

ZIO has the below logError function, which will automatically log if the this ZIO element is an error.

  /**
   * Logs the cause of failure of this workflow.
   */
  final def logError(implicit trace: Trace): ZIO[R, E, A] =
    logError("")

Effect also has a logError function which works completely differently. That's fine, but it would also be nice to have a function like the one above which will just automatically log any error for me.

What alternatives have you considered?

      .pipe(Effect.tapError(e => Effect.logError(`name='${e.name}' message='${e.message}'`)))

No response

@decoursin decoursin added the enhancement New feature or request label Apr 30, 2024
@jessekelly881
Copy link
Contributor

I think Loggers should probably just be extended to have better support for Error types so that.pipe(Effect.tapError(Effect.logError)) is sufficient.

@decoursin
Copy link
Author

I would think it not to be best to have the logError function have two purposes. Right now logError means logging at the error level. Or otherwise that should then be done for all of the logging functions like logDebug, logInfo, etc or for none of them. I don't think logError should be special.

@jessekelly881
Copy link
Contributor

jessekelly881 commented Apr 30, 2024

That's what I mean, yeah. I think all loggers, logDebug, etc. should have better support for logging an instance of an Error type if that's what's provided to it. #2667

@decoursin
Copy link
Author

Ah, ok! I think that could be cool, yeah!

@datner
Copy link
Member

datner commented May 2, 2024

They already have good support for Cause

effect.pipe(Effect.tapErrorCause(Effect.logError))

@MrOxMasTer
Copy link

I'm stressed about how the functions in Effect are sometimes done. Why the question when using a normal .tap, I can return nothing and everything will work fine:
image
image

But if I want to make a custom function, I will definitely have to return them again....
image
image

It seems a bit contrary to the ideology of .tap* functions or I don't understand something

@datner
Copy link
Member

datner commented Oct 21, 2024

@MrOxMasTer there is no "ideology of .tap* functions", Effect.tap-- and only it, no extrapolations, has the same usage ergonomics like Effect.andThen has, just with its take-left semantics.
This is so users who prefer Effect.andThen can use Effect.tap with the same intuition.
This is rather besides the point though, if you have questions we can chat over discord

@MrOxMasTer
Copy link

MrOxMasTer commented Oct 22, 2024

ther besides the point though, if you have questions we can chat over discord

There is only one question. Why isn't there a function that would also look through errors (for example, for logging) and they wouldn't have to be returned.

If you look at the words from the documentation: Executes an effectful operation to inspect the failure of an effect without altering it., then it should not alter errors, but:

image

I can change the number of errors, but not the errors themselves of course. This seems both thoughtful and strange at the same time

@MrOxMasTer
Copy link

MrOxMasTer commented Oct 22, 2024

You can of course do it like this
image

but, for customization you will need an explicit error return:

Effect.tapError((error) => {

// Your own code

return Effect.fail(error)
})

@datner
Copy link
Member

datner commented Oct 22, 2024

@MrOxMasTer

I can change the number of errors

You cannot. It will be only 1 or 0. A union is not an array. What you accumulate is the type of a potential error.

If you look at the words from the documentation: Executes an effectful operation to inspect the failure of an effect without altering it., then it should not alter errors

It does not alter the error. What you're experiencing is erroring while processing an error. Analogue to saying that this function is 'altering the error'

function bar() {
  throw new Error("dang")
}

try {
  foo()
} catch (e) {
  bar()
}

The error is effectively swallowed in these conditions, but that's expected.

What you're probably confused about is the track switching. You don't need to return a failed Effect just because you're working on the failed channel. Effect.logError is not a special member, it sends your message to the Logger and returns Effect.void. Effect.void is not a failing effect or a special effect, it's just a noop. Succeeding with nothing.

Effect.tapError((error) => {

// Your own code

// you can return any effect you want to execute.
// its value would be discarded, like `tap`.
// Its failures would halt execution, like `tap`.
// except that it resolving successfully does not count as 'recovering' from the failure.
return Effect.succeed(10)
return Effect.sync(() => {..})
return Effect.flatMap(someEffect, someOtherEffect)
return Effect.void 
})

Now I will ask again, please if you have any questions reach out to our discord. We are being unfair to the OP and collaborators by hijacking the thread like this, I would not answer any followups here, I will gladly do so on discord.

Hope this clears things up a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants