1
\$\begingroup\$

I have written a simple API client for my app in Go. The client is very simple, it wraps http.Client and has some convenience methods.

The one method I am trying to get some feedback on, does endpoint polling with timeout. It blocks, and returns when either a timeout is reached or status returned by API is successful or in error.

My questions are in comments of the function itself. I have omitted some parts (structs, the "constructor method") that are not relevant to discussion.

caller (in main)

log.Printf("polling status, id: %d", sid)
status, err := api.PollStatus(sid, 1*time.Second, 5*time.Second)
if err != nil {
    log.Fatalf("PollStatus error: %+v", err)
}

log.Print(status)

the method

// PollStatus will keep 'pinging' the status API until timeout is reached or status returned is either sucesfull or in error.
// NOTES:
//   - this function will block the caller
// QUESTIONS:
//  - is the design/functionality of this method idiomatic Go or is there a better way?
//  - do I need to close ticker and timeout channels? If so how?
//  - is it safe to 'defer ticker.Stop()' at the start of goroutine or do I have to call it before each return?
func (client *APIClient) PollStatus(id int, pollInterval time.Duration, pollTimeout time.Duration) (*Status, error) {
    var status *Status
    var err error
    var wg sync.WaitGroup
    ticker := time.NewTicker(pollInterval)
    timeout := time.After(pollTimeout)
    tickerCounter := 0

    // increment the WaitGroup counter for number of goroutines to wait for...
    wg.Add(1)

    go func() {
        defer ticker.Stop()
        // decrement the counter when the goroutine completes.
        defer wg.Done()

        for {
            select {

            case tout := <-timeout:
                log.Printf("polling timeout of '%s' reached at '%s'...", pollTimeout, tout.Format(time.RFC3339))
                //ticker.Stop()
                return

            case tick := <-ticker.C:
                status, err = client.GetStatus(id)
                tickerCounter++
                if err != nil {
                    //ticker.Stop()
                    return
                }
                log.Printf("tick #%d at '%ss', %s", tickerCounter, tick.Format("05"), status)
                if status.IsErrorStatus() {
                    err = errors.New("error while polling for status")
                    //ticker.Stop()
                    return
                }
                if status.IsCompletedStatus() {
                    err = nil
                    //ticker.Stop()
                    return
                }

            }
        }
    }()

    // wait for polling to complete (or time out)
    wg.Wait()

    if err != nil {
        return nil, err
    }
    if status == nil {
        return nil, errors.New("unable to get status")
    }

    return status, nil
}
\$\endgroup\$
2
  • \$\begingroup\$ First thing that comes to mind is: Why aren't you using context.Context, you can use a context with timeout out of the box. See the docs \$\endgroup\$ May 2, 2017 at 20:12
  • \$\begingroup\$ @EliasVanOotegem I was using this answer from SO as a guide...But if context would be better how can I "wrap" it into my method... \$\endgroup\$
    – zam6ak
    May 2, 2017 at 21:21

1 Answer 1

1
\$\begingroup\$

How about this? I used context.Context with the deadline which also gives control to the caller of poll if you want to cancel the poll before the deadline through CancelFunc. I do not have a full context of the problem here, but this should help you.

// PollStatus will keep 'pinging' the status API until timeout is reached or status returned is either successful or in error.
func (client *APIClient) PollStatus(ctx context.Context, id int, pollInterval time.Duration) (*Status, error) {
        ticker := time.NewTicker(pollInterval)
        defer ticker.Stop()
        tickerCounter := 0
        for {
            select {
            case <-ctx.Done():
                return nil, ctx.Err()

            case tick := <-ticker.C:
                status, err := client.GetStatus(id)
                tickerCounter++
                if err != nil {
                    return nil, err
                }

                log.Printf("tick #%d at '%ss', %s", tickerCounter, tick.Format("05"), status)
                if status.IsErrorStatus() {
                    return nil, errors.New("error while polling for status")
                }

                if status.IsCompletedStatus() {
                    return status, nil
                }

            }
        }

        return nil, errors.New("unable to get status")
  }

func someFunc() {
    log.Printf("polling status, id: %d", sid)
    ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
    // Having cancelFunc here gives control if you want to cancel the poll before deadline
    // but for this instance, I'm just gonna defer cancel it
    defer cancelFunc()
    status, err := api.PollStatus(ctx, sid, 1*time.Second)
    if err != nil {
        log.Fatalf("PollStatus error: %+v", err)
    }

    log.Print(status)
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.