RobotsAndPencils / buford

A push notification delivery engine for the new HTTP/2 APNS service.
MIT License
475 stars 52 forks source link

queue.Responses not executed for the last response #74

Closed roma86 closed 8 years ago

roma86 commented 8 years ago

What did you do? (steps to reproduce or a code sample is helpful)

➜  go go version
go version go1.6.2 darwin/amd64

➜  go go get -u -d github.com/RobotsAndPencils/buford
➜  go
package main

import(
  "encoding/json"
  "log"
  "crypto/tls"

  "github.com/RobotsAndPencils/buford/payload"
  "github.com/RobotsAndPencils/buford/payload/badge"
  "github.com/RobotsAndPencils/buford/push"
)

const (
  certFile = "../promiser-apns.pem"
  keyFile = "../promiser-apns.key"
  host = push.Development
  deviceToken = "afc61ee7a0d29ddbf68c7299017a549880f50bcb9a9b5e1a45c21dbfb064d047"
)

func main() {
  // load a certificate and use it to connect to the APN service:
  if cert, err := tls.LoadX509KeyPair(certFile, keyFile); err == nil {

    if client, err := push.NewClient(cert); err == nil {

      // construct a payload to send to the device:
      payloadObj := payload.APS{
        Alert: payload.Alert{Body: "Hello HTTP/2"},
        Badge: badge.New(42),
      }

      if payloadData, err := json.Marshal(payloadObj); err == nil {

        service := push.NewService(client, host)
        queue := push.NewQueue(service, 5)

        go func() {
          for resp := range queue.Responses {
            log.Println("++++>", resp)
          }
        }()

        headers := &push.Headers{
          Topic: "com.letsapp.BundleID",
        }

        // send the notifications
        for i := 0; i < 5; i++ {
          queue.Push(deviceToken, headers, payloadData)
        }

        // done sending notifications, wait for all responses and shutdown
        queue.Wait()

      } else {
        log.Printf("init client error: %v \n", err)
      }

    } else {
      log.Printf("init client error: %v \n", err)
    }

  } else {
    log.Printf("load cert error: %v \n", err)
  }
}

What did you expect to see?

2016/07/22 22:44:36 ++++> {token id error_nil}
2016/07/22 22:44:37 ++++> {token id error_nil}
2016/07/22 22:44:38 ++++> {token id error_nil}
2016/07/22 22:44:39 ++++> {token id error_nil}
2016/07/22 22:44:40 ++++> {token id error_nil}

What did you see instead?

2016/07/22 22:44:36 ++++> {token id error_nil}
2016/07/22 22:44:37 ++++> {token id error_nil}
2016/07/22 22:44:38 ++++> {token id error_nil}
2016/07/22 22:44:39 ++++> {token id error_nil}
nathany commented 8 years ago

Thanks for the report. Will look into it.

nathany commented 8 years ago

I'm definitely seeing this issue using the -n flag to example/concurrent. Sometimes all the responses come back, sometimes not. Still investigating.

go run main.go -c cert.p12 -d device_token -n 20

Are you also finding that sometimes you receive all responses and other times not?

nathany commented 8 years ago

Right now calling queue.Wait() waits until all the responses are sent (which also means all responses are received).

However, it's possible that the goroutine reading the responses hasn't finished yet when the program exits.

I added some logging introduced a time.Sleep before exiting and occasionally saw this behaviour:

17:20:38.065437 (19) apns-id: D989EEA8-37F8-A681-B578-B4C6F0FC327F
17:20:38.065448 response sent 5ABCCCD8-1034-59AD-78C9-680199E0687B
17:20:38.065458 done
17:20:38.065475 (20) apns-id: 5ABCCCD8-1034-59AD-78C9-680199E0687B

the final response is sent, queue.Wait() wraps up, and the program would then exit -- but thanks to sleeping for a second, we do see the final response.

the proper solution is to add a WaitGroup or done channel to your own code. see #75