Use of log.Fatal causes upstream application to exit

[jrschumacher]

The use of log.Fatal causes the upstream application to exit.

oidc/pkg/http/http.go

Lines 95 to 112 in c3c1bd3

	func StartServer(ctx context.Context, port string) {
	server := &http.Server{Addr: port}
	go func() {
	if err := server.ListenAndServe(); err != http.ErrServerClosed {
	log.Fatalf("ListenAndServe(): %v", err)
	}
	}()
	go func() {
	<-ctx.Done()
	ctxShutdown, cancelShutdown := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancelShutdown()
	err := server.Shutdown(ctxShutdown)
	if err != nil {
	log.Fatalf("Shutdown(): %v", err)
	}
	}()
	}

Is there a reason why we don't want to return an error and/or the shutdown function and expect developers to implement the go-routine to shutdown appropriately?

log.Fatal will kill the process even in goroutines https://go.dev/play/p/Hy6rKkePzJY

[muhlemmer]

Can you be specific about which log.Fatal use is bugging you?

[jrschumacher]

@muhlemmer my bad I posted the missing permalink in the original message

[muhlemmer]

As far as I can tell, StartServer is only used for CLI applications. We use the global http.DefaultServeMux

If in a CLI StartServer fails, would you still want to resume normal execution? Sounds unlikely to me. If that is important for you, we are open for a PR which handles the error more gracefully. If you do so, make sure to create a new function and don't touch the old one to prevent breaking changes as per semver.

[jrschumacher]

We use it for a desktop application built with https://github.com/wailsapp/wails. In the random chance that the IdP timesout an error will be raised and the login server log a fatal error that executes os.Exit(). If it were to panic we'd have captured it with recover. (We also use it for our CLI app, but like you said don't notice it although if we were building a TUI with Bubbletea we'd probably experience it.

Sounds good. I'll work on a PR this week.