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

Use log.Print, not log.Fatal in the main #14

Open
sofasurfa opened this issue Oct 22, 2020 · 2 comments
Open

Use log.Print, not log.Fatal in the main #14

sofasurfa opened this issue Oct 22, 2020 · 2 comments

Comments

@sofasurfa
Copy link

sofasurfa commented Oct 22, 2020

@spy16 Also I noticed that defer closeSession() wouldn't execute in the main.
As Fatalf logs the error and then calls os.Exit(1).

defer closeSession()

if err := srv.ListenAndServe(); err != nil {
		lg.Fatalf("http server exited: %s", err) // os.Exit(1) doesn't care about defer
	}

I think it's better to do this

if err := srv.ListenAndServe(); err != nil {
		lg.Printf("http server exited: %s", err) // Print instead
}
defer os.Exit(1)
defer closeSession() // this executes first 

Another option would be to use panic() instead of lg.Fatalf() every where, and then at the bottom of main we could recover panics...

db, closeSession, err := mongo.Connect(cfg.MongoURI, true)
if err != nil {
  panic("failed to connect to mongodb: " + err)
}

// do the same for server and every where else
if err := srv.ListenAndServe(); err != nil {
		panic("http server exited: " + err)
}

// Recover all panics 
defer func() {
        if r := recover(); r != nil {
            closeSession()
            //.. add more clean ups here if needed (we have access to all the functions declared  higher)
            lg.Fatalf(r)  
          
        }
  }()

This methods creates a centralised place for our clean ups, but it does require to use panic for every error that occurs in the main. So it's not very clean, and it kind of goes against the purpose of panic... thus I think method 1 is better.
What do you think?

@sofasurfa sofasurfa changed the title Use log.Print, not log.Fatal Use log.Print, not log.Fatal in the main Oct 22, 2020
@sofasurfa
Copy link
Author

@spy16 Created a pull request #15

@spy16
Copy link
Owner

spy16 commented Feb 21, 2021

Apologies for the delay and thanks for the PR. Left a comment on the PR. Do take a look and let me know your thoughts.

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

No branches or pull requests

2 participants