Fatal should deterministically calculate the POSIX return code

[cardil]

The Fatal method is calling os.Exit(1) regardless of a message. This isn't the best UX choice. The POSIX return code could be used for quick assessment about a process failure. When the retcode is always equal 1 for any error, this can't be utilized.

Tools like Docker or Kubernetes report back the return code of the process. By using a deterministic algorithm to calculate the POSIX return code, a quick debugging is possible. Users could indicate if the process is failing because of the same reason, without looking into logs.

The best would be to rely on Fatal's message or on an error object if it's attached as a Field.

[sywhang]

Hi there 👋 thank you for the contribution, but this is a breaking change. Fatal calling os.Exit(1) is a documented behavior that shouldn't change unconditionally.

That being said, you should be able to change the behavior of Fatal by using the OnFatal Option and write your own handler for it that returns a more deterministic error code.

[prashantv]

+1 on the backwards compatibility. However, the OnFatal option can't be used to implement a custom handler -- it's an enum, so it's not possible to implement this with zap today. Supporting a way to specify custom exit logic (or exit code) seems like a useful addition, so this sort of logic could be implemented outside of zap.

[cardil]

I think we could change the OnFatal to be a function instead of enum. Such change should be compatible for end-users. They will continue using CheckWriteAction names like: WriteThenGoexit, WriteThenPanic and WriteThenFatal, but they will be functions instead.

[cardil]

I updated the #1088 to be backward compatible, by adding new CheckWriteAction of WriteThenPosixExitCode.

The new action will calculate the os.Exit return code from message or from error Field if given.

[abhinav]

@cardil, thanks for the report and the contribution! We realize that this is something that might deserve a customizable. Unfortunately, the current implementation in #1088 isn't fit to merge because:

• Converting CheckWriteAction from an integer to a function is not a backwards compatible change
• "Hash of the message" is a deterministic scheme, but my gut feeling is that it is not obvious for a user

I think I would lean more towards a variant where we support an arbitrary func hook that deprecates the "OnFatal" option.

This requires a little care to do in a backwards compatible way but, @prashantv @sywhang @cardil, what do you think of something like the following?

package zap

func WithFatalHook(zapcore.CheckWriteHook) Option

package zapcore

type CheckWriteHook interface {
  OnWrite(*CheckedEntry, []Field)
}

func (*CheckedEntry) After(Entry, CheckWriteHook) *CheckedEntry

• In package zap, the OnFatal option will be deprecated in favor of WIthFatalHook. Existing uses will proxy into WithFatalHook equivalents.
• In package zapcore, CheckedEntry.Should will similarly be deprecated for a CheckedEntry.After(Entry, CheckWriteHook).

To make the deprecation easy, zapcore.CheckWriteAction will implement the CheckWriteHook interface so a user will be able to do zap.WithFatalHook(zapcore.WriteThenGoexit)--same as what they can do with zap.OnFatal.

[cardil]

Thanks, @abhinav, for your opinion. I like it!

I think we could go without any deprecations. Take a look at my updated PR: #1088. It incorporates your ideas, and allows keeping current code, and customize the action as well.

[abhinav]

Thanks for the PR @cardil, and thanks @sywhang for the tweaks.
@cardil we've merged the PR after removing the new option.
We think it makes sense for Zap to support arbitrary Fatal-message hooks,
but we didn't want to hard-code the logic to "exit code is calculated by hashing the message".
We agree that that behavior was deterministic, but we were concerned that it might also be surprising.
Instead of making the decision for users, we decided that the WithFatalHook option alone should suffice.
A user can provide a hash-based CheckWriteHook when building the logger and use that everywhere.

Hope this helps, and thanks again for your contribution.