63

I have so far avoided use of log.Fatal, but I recently co-incidentally discovered these questions; code-coverage and tests-using-log-fatal.

One of the comments from the 100 code coverage questions says:

... In the vast majority of cases log.Fatal should be only be used in main, or init functions (or possibly some things meant to be called only directly from them)"

It go me thinking, so I began to look at the standard library code provided with Go. There are lots of examples where the test code in the library makes use of log.Fatal which seems fine. There are a few examples outside of the test code, such as in net/http, shown below:

// net/http/transport.go 
func (t *Transport) putIdleConn(pconn *persistConn) bool {
    ...
    for _, exist := range t.idleConn[key] {
        if exist == pconn {
            log.Fatalf("dup idle pconn %p in freelist", pconn)
        }
    }
    ...
}

If its is best practice to avoid use of log.Fatal, why is it used at all in the standard libraries, I would have expected just return an error. It seems unfair to the user of the library to cause os.Exit to be called and not providing any chance for the application to clean-up.

I may be naive, so hence my question as a better practice would seem to be to call log.Panic which can be recovered and my theoretical long running stable application might have a chance of rising from the ashes.

So what would best-practise say for Go about when should log.Fatal should be used?

Community
  • 1
  • 1
miltonb
  • 6,905
  • 8
  • 45
  • 55
  • 6
    My hope would be that this code is absolutely unreachable. That an error that causes an idle connection to be still in the idle list while also being used as an active connection should not be something that should happen ever, and for it to occur should be something catastrophic. But since they appear to be using mutexes properly around the idle list and so forth, I have no idea why this loop and code would even be necessary. Why they would further exit your program immediately rather than panicing is another mystery. Great question. – captncraig Nov 24 '15 at 04:09
  • Are any of the tests in that package able to reach that line? – captncraig Nov 24 '15 at 04:12
  • Good question, had a little look and cant see anything that looks like its specifically designed to reach that line. Not conclusive yet ... – miltonb Nov 24 '15 at 04:19
  • 6
    "Occasionally used in the standard library" doesn't conflict with "best practice" - some of the std. lib was certainly written before best practices/idioms emerged. I would argue that in any *new* application, only a `package main` should call `log.Fatal`. – elithrar Nov 24 '15 at 04:21
  • Some circumstances _are_ fatal and continuing might be worse than aborting. Returning an error and hopeing the caller will react properly is often advisable, but assume some crypto routines detect that the random source is corrupted: Most probably you should not do much at all after this discovery except shred the hardware. log.Fatal seems appropriate here. – Volker Nov 24 '15 at 07:48
  • 1
    I'd say that since `log.Fatal()` calls `os.Exit(1)` in the end, you should not do that—use plain `panic()` instead: if it won't be caught, the application will terminate anyway, with nice stack traces printed, and if it will be caught, any panic emerged from a package's code means all bets are off anyway. So panicking just offers greater room for processing it to the package users. – kostix Nov 24 '15 at 07:49
  • I do agree that it does not conflict with best practice. @kostix your approach seems to be easier for the user of the package to have some semblance of controlled failure. I also now consider that my concept of 'rising from the ashes' is not useful, it is desirable though to have a controlled failure from main (not package code) which then ends execution. – miltonb Nov 24 '15 at 20:03
  • It's used in the golang grpc example too: https://grpc.io/docs/quickstart/go.html – limscoder May 11 '18 at 19:57

3 Answers3

73

It might be just me, but here is how I use log.Fatal. As per UNIX conventions, a process which encounters an error should fail as early as possible with a non-zero exit code. This lead me to the following guidelines to use log.Fatal when…

  1. …an error happens in any of my func init(), as these happen when the imports are processed or before the main func is called, respectively. Conversely, I only do stuff not directly affecting the unit of work the library or cmd is supposed to do. For example, I set up logging and check wether we have a sane environment and parameters. No need to run main if we have invalid flags, right? And if we can not give proper feedback, we should tell this early.
  2. …an error happens of which I know is irrecoverable. Let's assume we have a program which creates a thumbnail of an image file given on the command line. If this file does not exist or is unreadable because of insufficient permissions, there is no reason to continue and this error can not be recovered from. So we adhere to the conventions and fail.
  3. …an error occurs during a process which might not be reversible. This is kind of a soft definition, I know. Let me illustrate that. Let's assume we have an implementation of cp, and it was started to be non-interactive and recursively copy a directory. Now, let's assume we encounter a file in the target directory which has the same name (but different content) as a file to be copied there. Since we can not ask the user to decide what to do and we can not copy this file, we have a problem. Because the user will assume that the source and the target directories are exact copies when we finish with exit code zero, we can not simply skip the file in question. However, we can not simply overwrite it, since this might potentially destroy information. This is a situation we can not recover from per explicit request by the user, and so I'd use log.Fatal to explain the situation, hereby obeying the principle to fail as early as possible.
Markus W Mahlberg
  • 19,711
  • 6
  • 65
  • 89
  • 9
    I like your answer as it explains the points well (and resonates with my own ideas on this topic) but I'm afraid it misses a crucial bit: the OP explicitly asked about using `log.Fatal` *in a package* -- that is, in a piece of code not controlled by whoever wrote `main()`. As you can see, this actually moves the question from a "well-behaving process" domain to the "well-behaving package" domain -- quite a different story for the question becomes: whether it's okay to irreversively fail someone else's program? – kostix Nov 24 '15 at 13:34
  • 4
    It is covered, albeit a bit implicit: I apply those rules regardless of wether I write a package or a cmd: an irrecoverable error is an irrecoverable error. It is a programs responsibility to give a package sanitized input. If that isn't possible, an error should be returned, but only if the input can not be sanitized beforehand. So fataling where appropriate actually helps the user of an package to write better code. – Markus W Mahlberg Nov 24 '15 at 13:40
  • 10
    Thing is, it is not the package maintainer's call if something is irrecoverable or not. Sure, you failed to generate a thumbnail, but I was using that as a small part of my web server. If a thumbnail package calls os.Exit because it can't load a file, I would be incensed. – captncraig Nov 25 '15 at 15:21
  • Well, that's of course true and the picture example was explicitly stated as per command line. The go packages I have created thus far are for my tools only, where I pass the logger around. Given that log.Logger isn't an interface and Logger is rather limited, I don't think I would do **any** logging in public packages (users might use incompatible loggers such as logxi) and work with returned errors only. \*still scratching my head on the decision not to make Logger an interface\* – Markus W Mahlberg Nov 25 '15 at 15:28
  • Totally agree . The fatal stops the program flow early enough to avoid continuity – I.Tyger Nov 27 '18 at 15:46
10

Marcus, I came across your response, and I think its excellent and very insightful, and I tend to agree with your breakdown. It is very hard to generalize, though I have been thinking about it this a little more, and as a newbie to Go. I think on a theoretical level, if we are looking to best practices in computing, regardless of OS, package framework or library, a logger's responsibility is to simply log. On any level, a logger's responsibilities:

  • Format and print information consistently to the channel of my choosing.
  • Categorize, filter, display the different log levels [debug, info, warn, error]
  • Handle and aggregate log entries across async and parallel jobs

A logging package or any package does not and should not have authority to crash a program, if it is operating properly. Any middleware or library should follow a throw / catch pattern, with an opportunity for all exceptions thrown to be caught by the caller. This is also a good pattern to follow within an application, as you build foundations and packages that power various parts of your application, and potentially other applications, they should never crash an application directly. Rather, they should throw a fatal exception, allowing the program to handle. I think this also addresses some of your points Marcus, as it works to alert the caller immediately when uncaught, as a fatal crash.

For the most part I can enjoy leveraging log.Fatal in Go directly for the purposes of direct user facing cli tools, which I think is the simplicity it was truly intended for. I don't think it makes good sense as a long term approach to handle fatal errors across packages.

moniecodes
  • 101
  • 1
  • 2
  • 1
    I agree with @moniecodes: packages should not crash your program, even if an error is deemed "unrecoverable" because a package does not have sufficient context to know what else in the caller needs to be cleaned up before an exit. – jgpawletko Jul 24 '20 at 18:57
  • 1
    @jgpawletko "Crash"? Stopping processing because of good reasons is not exactly a "crash". – Markus W Mahlberg Dec 12 '21 at 11:08
3

log.Fatal makes use of os.Exit and is best called when an error is irreversible and may affect the entire program. I think log.Panic is a more lenient option.

E. Francis
  • 98
  • 6