Should a program always exit in main()?

244 views
Skip to first unread message

Francis Chuang

unread,
Feb 7, 2017, 6:12:13 PM2/7/17
to golang-nuts
I am working on a client library that starts a Kafka consumer to listen for messages from a kafka stream and run some user defined handlers. I want the design to be "crash-only" as much as possible, so will be avoiding cleanups during unexpected shutdowns.

For example, here's a simple sketch:

func myHandler(m client.Message) error{
...
}

c := client.New(...)
c.AddHandler(myHandler)
c.Start()

c.Start() looks like this:

func (c *Client) Start(){
  
   for{
     select{

       case m := <-c.message:
        
         for _, handler := range c.handlers{
              err := handler(m)
         }
    }
   }
}

If the handler encounters an error, I have the following options:
- call log.Fatal() within the for _, handler := range loop. This seems to be the simplest, but it doesn't seem right for a library to exit.
- remove the return error from the handler, and call log.Fatal() within so that the handler terminates the whole program.
- create an error channel, have the for _, handler := range loop write errors to that channel. In main(), have a for, select loop to read from the error channel and call log.Fatal() there.

Which of these options is the most idiomatic?

Cheers,
Francis

Christoph Berger

unread,
Feb 8, 2017, 4:23:38 AM2/8/17
to golang-nuts
To me, the simplest way seems to be to just return an error to the caller outside the library:

func (c *Client) Start() error {

for {
select {

case m := <-c.message:

for _, handler := range c.handlers {
err := handler(m)
if err != nil {
   return err
}

}
}
}
}

Francis Chuang

unread,
Feb 8, 2017, 5:04:15 AM2/8/17
to golang-nuts
I made a mistake with my example. c.Start() needs to be a separate go routine because it is an infinite loop waiting for messages: go c.Start().

In that case, I am guessing an errors channel would be the best?

Cheers,
Francis

Axel Wagner

unread,
Feb 8, 2017, 5:38:20 AM2/8/17
to Francis Chuang, golang-nuts
It's not uncommon to endless-loop until something goes wrong in such a case (see, for example, http.Serve). Just return an error from Client.Start as suggested and then catch that in main and log.Fatal there. If a user of your library needs to do more code, they'll have to wrap the concurrent error handling code themselves.

I wouldn't expose channels in an API if at all preventable. They usually open up questions that the compiler can't check and that, if answered correctly, will result in mostly the equivalent amount of code to just wrapping a synchronous API anyway. For example: Does the channel need to be buffered or not? What would a good channel size be? What happens if the reader from the channel is slow, do values get dropped, or queued, or does the sender block? If the latter, is there a timeout?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Christoph Berger

unread,
Feb 8, 2017, 6:20:37 AM2/8/17
to Francis Chuang, golang-nuts
Well, I already thought that my suggestion seems somewhat too simple… :)

I agree, if a fatal error occurs within a goroutine but you don’t want to crash the whole process from within this goroutine (or even from within the library), then error channels seem the way to go.

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/zssLR41Kk3Q/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

Eric Johnson

unread,
Feb 8, 2017, 6:40:31 PM2/8/17
to golang-nuts


On Tuesday, February 7, 2017 at 3:12:13 PM UTC-8, Francis Chuang wrote:
I am working on a client library that starts a Kafka consumer to listen for messages from a kafka stream and run some user defined handlers. I want the design to be "crash-only" as much as possible, so will be avoiding cleanups during unexpected shutdowns.

That's sort of a curious model. Your description would make sense if you were describing an application, but you're describing a library. By building that design assumption into your library, you're limiting the possible uses of the library. That approach also makes it incredibly difficult to write unit-tests for your code.
 

For example, here's a simple sketch:

func myHandler(m client.Message) error{
...
}

c := client.New(...)
c.AddHandler(myHandler)
c.Start()

c.Start() looks like this:

func (c *Client) Start(){
  
   for{
     select{

       case m := <-c.message:
        
         for _, handler := range c.handlers{
              err := handler(m)
         }
    }
   }
}

If the handler encounters an error, I have the following options:
- call log.Fatal() within the for _, handler := range loop. This seems to be the simplest, but it doesn't seem right for a library to exit.
- remove the return error from the handler, and call log.Fatal() within so that the handler terminates the whole program.
- create an error channel, have the for _, handler := range loop write errors to that channel. In main(), have a for, select loop to read from the error channel and call log.Fatal() there.

Only the last one seems acceptable for logic in a library. Because the decision to exit the program actually no longer belongs to the library, but to "main()", or a function called from main(). Not even clear to me that you want to call log.Fatal() there, either. Since you're talking about multiple Go routines, it sounds like you may have a race condition where multiple errors may have occurred. By calling log.Fatal(), you're guaranteeing that only one of those errors will be printed. The error message you may get may not be the most useful or relevant one, depending on the errors, and the race condition problems.

Instead it seems like you might want to do proper cleanup of all the launched goroutines, and then drain the error channel in main(), and then exit. That way, you get all the error messages.
 

Which of these options is the most idiomatic?

Exiting the program abruptly is not idiomatic. The one that moves that decision closest to the "main()" function seems like the best idea.

Eric.
 

Cheers,
Francis
Reply all
Reply to author
Forward
0 new messages