Skip to content

Commit

Permalink
Remove error handler from codei (use feedback) (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
@capella @cscatolini
capella authored and cscatolini committed Apr 30, 2019
1 parent de5d642 commit a81fc73
Show file tree
Hide file tree
Showing 12 changed files with 2 additions and 457 deletions.
7 changes: 0 additions & 7 deletions extensions/apns_message_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ type APNSMessageHandler struct {
failuresReceived int64
feedbackReporters []interfaces.FeedbackReporter
InflightMessagesMetadata map[string]interface{}
InvalidTokenHandlers []interfaces.InvalidTokenHandler
IsProduction bool
Logger *log.Logger
LogStatsInterval time.Duration
Expand All @@ -87,7 +86,6 @@ func NewAPNSMessageHandler(
pendingMessagesWG *sync.WaitGroup,
statsReporters []interfaces.StatsReporter,
feedbackReporters []interfaces.FeedbackReporter,
invalidTokenHandlers []interfaces.InvalidTokenHandler,
pushQueue interfaces.APNSPushQueue,
) (*APNSMessageHandler, error) {
a := &APNSMessageHandler{
Expand All @@ -100,7 +98,6 @@ func NewAPNSMessageHandler(
failuresReceived: 0,
feedbackReporters: feedbackReporters,
InflightMessagesMetadata: map[string]interface{}{},
InvalidTokenHandlers: invalidTokenHandlers,
IsProduction: isProduction,
Logger: logger,
pendingMessagesWG: pendingMessagesWG,
Expand Down Expand Up @@ -286,10 +283,6 @@ func (a *APNSMessageHandler) handleAPNSResponse(responseWithMetadata *structs.Re
if responseWithMetadata.Metadata != nil {
responseWithMetadata.Metadata["deleteToken"] = true
}
handleInvalidToken(
a.InvalidTokenHandlers, responseWithMetadata.DeviceToken,
a.appName, "apns",
)
case apns2.ReasonBadCertificate, apns2.ReasonBadCertificateEnvironment, apns2.ReasonForbidden:
l.WithFields(log.Fields{
"category": "CertificateError",
Expand Down
26 changes: 1 addition & 25 deletions extensions/apns_message_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ var _ = FDescribe("APNS Message Handler", func() {
var db interfaces.DB
var feedbackClients []interfaces.FeedbackReporter
var handler *APNSMessageHandler
var invalidTokenHandlers []interfaces.InvalidTokenHandler
var mockKafkaProducerClient *mocks.KafkaProducerClientMock
var mockPushQueue *mocks.APNSPushQueueMock
var mockStatsDClient *mocks.StatsDClientMock
Expand Down Expand Up @@ -77,9 +76,6 @@ var _ = FDescribe("APNS Message Handler", func() {
feedbackClients = []interfaces.FeedbackReporter{kc}

db = mocks.NewPGMock(0, 1)
it, err := NewTokenPG(config, logger, db)
Expect(err).NotTo(HaveOccurred())
invalidTokenHandlers = []interfaces.InvalidTokenHandler{it}

mockPushQueue = mocks.NewAPNSPushQueueMock()
handler, err = NewAPNSMessageHandler(
Expand All @@ -94,7 +90,6 @@ var _ = FDescribe("APNS Message Handler", func() {
nil,
statsClients,
feedbackClients,
invalidTokenHandlers,
mockPushQueue,
)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -133,8 +128,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
Expect(hook.Entries).To(ContainLogMessage("deleting token"))
//Expect(hook.Entries[len(hook.Entries)-2].Data["category"]).To(Equal("TokenError"))
})

It("if response has error push.ErrBadDeviceToken", func() {
Expand All @@ -146,8 +139,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
Expect(hook.Entries).To(ContainLogMessage("deleting token"))
//Expect(hook.Entries[len(hook.Entries)-2].Data["category"]).To(Equal("TokenError"))
})

It("if response has error push.ErrBadCertificate", func() {
Expand All @@ -159,7 +150,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("CertificateError"))
})

It("if response has error push.ErrBadCertificateEnvironment", func() {
Expand All @@ -171,7 +161,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("CertificateError"))
})

It("if response has error push.ErrForbidden", func() {
Expand All @@ -183,7 +172,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("CertificateError"))
})

It("if response has error push.ErrMissingTopic", func() {
Expand All @@ -195,7 +183,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("TopicError"))
})

It("if response has error push.ErrTopicDisallowed", func() {
Expand All @@ -207,7 +194,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("TopicError"))
})

It("if response has error push.ErrDeviceTokenNotForTopic", func() {
Expand All @@ -219,7 +205,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("TopicError"))
})

It("if response has error push.ErrIdleTimeout", func() {
Expand All @@ -231,7 +216,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("AppleError"))
})

It("if response has error push.ErrShutdown", func() {
Expand All @@ -243,7 +227,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("AppleError"))
})

It("if response has error push.ErrInternalServerError", func() {
Expand All @@ -255,7 +238,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("AppleError"))
})

It("if response has error push.ErrServiceUnavailable", func() {
Expand All @@ -267,7 +249,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("AppleError"))
})

It("if response has untracked error", func() {
Expand All @@ -279,7 +260,6 @@ var _ = FDescribe("APNS Message Handler", func() {
handler.handleAPNSResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
//Expect(hook.LastEntry().Data["category"]).To(Equal("DefaultError"))
})
})

Expand Down Expand Up @@ -425,9 +405,7 @@ var _ = FDescribe("APNS Message Handler", func() {
feedbackClients = []interfaces.FeedbackReporter{kc}

db = mocks.NewPGMock(0, 1)
it, err := NewTokenPG(config, logger, db)
Expect(err).NotTo(HaveOccurred())
invalidTokenHandlers = []interfaces.InvalidTokenHandler{it}

handler, err = NewAPNSMessageHandler(
authKeyPath,
keyID,
Expand All @@ -440,7 +418,6 @@ var _ = FDescribe("APNS Message Handler", func() {
nil,
statsClients,
feedbackClients,
invalidTokenHandlers,
mockPushQueue,
)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -598,7 +575,6 @@ var _ = FDescribe("APNS Message Handler", func() {
nil,
nil,
nil,
nil,
)
Expect(err).NotTo(HaveOccurred())
hook.Reset()
Expand Down
6 changes: 0 additions & 6 deletions extensions/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ type ParsedTopic struct {
Game string
}

func handleInvalidToken(invalidTokenHandlers []interfaces.InvalidTokenHandler, token string, game string, platform string) {
for _, invalidTokenHandler := range invalidTokenHandlers {
invalidTokenHandler.HandleToken(token, game, platform)
}
}

func getGameAndPlatformFromTopic(topic string) ParsedTopic {
res := topicRegex.FindStringSubmatch(topic)
return ParsedTopic{
Expand Down
32 changes: 0 additions & 32 deletions extensions/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@
package extensions

import (
"fmt"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
uuid "github.com/satori/go.uuid"
"github.com/spf13/viper"

"github.com/sirupsen/logrus"
Expand All @@ -41,8 +38,6 @@ var _ = Describe("Common", func() {
var config *viper.Viper
var mockKafkaProducerClient *mocks.KafkaProducerClientMock
var feedbackClients []interfaces.FeedbackReporter
var invalidTokenHandlers []interfaces.InvalidTokenHandler
var db *mocks.PGMock

configFile := "../config/test.yaml"
logger, hook := test.NewNullLogger()
Expand All @@ -57,36 +52,9 @@ var _ = Describe("Common", func() {
kc, err := NewKafkaProducer(config, logger, mockKafkaProducerClient)
Expect(err).NotTo(HaveOccurred())
feedbackClients = []interfaces.FeedbackReporter{kc}

db = mocks.NewPGMock(0, 1)
it, err := NewTokenPG(config, logger, db)
Expect(err).NotTo(HaveOccurred())
invalidTokenHandlers = []interfaces.InvalidTokenHandler{it}

hook.Reset()
})

Describe("Handle token error", func() {
It("should be successful", func() {
token := uuid.NewV4().String()
handleInvalidToken(invalidTokenHandlers, token, "test", "apns")
query := "DELETE FROM test_apns WHERE token = ?0;"
Expect(db.Execs).To(HaveLen(2))
Expect(db.Execs[1][0]).To(BeEquivalentTo(query))
Expect(db.Execs[1][1]).To(BeEquivalentTo([]interface{}{token}))
})

It("should fail silently", func() {
token := uuid.NewV4().String()
db.Error = fmt.Errorf("pg: error")
handleInvalidToken(invalidTokenHandlers, token, "test", "apns")
Expect(db.Execs).To(HaveLen(2))
query := "DELETE FROM test_apns WHERE token = ?0;"
Expect(db.Execs[1][0]).To(BeEquivalentTo(query))
Expect(db.Execs[1][1]).To(BeEquivalentTo([]interface{}{token}))
})
})

Describe("Send feedback to reporters", func() {
It("should return an error if res cannot be marshaled", func() {
badContent := make(chan int)
Expand Down
7 changes: 0 additions & 7 deletions extensions/gcm_message_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ type GCMMessageHandler struct {
feedbackReporters []interfaces.FeedbackReporter
GCMClient interfaces.GCMClient
InflightMessagesMetadata map[string]interface{}
InvalidTokenHandlers []interfaces.InvalidTokenHandler
IsProduction bool
Logger *log.Logger
LogStatsInterval time.Duration
Expand Down Expand Up @@ -90,7 +89,6 @@ func NewGCMMessageHandler(
pendingMessagesWG *sync.WaitGroup,
statsReporters []interfaces.StatsReporter,
feedbackReporters []interfaces.FeedbackReporter,
invalidTokenHandlers []interfaces.InvalidTokenHandler,
client interfaces.GCMClient,
) (*GCMMessageHandler, error) {
l := logger.WithFields(log.Fields{
Expand All @@ -106,7 +104,6 @@ func NewGCMMessageHandler(
failuresReceived: 0,
feedbackReporters: feedbackReporters,
InflightMessagesMetadata: map[string]interface{}{},
InvalidTokenHandlers: invalidTokenHandlers,
IsProduction: isProduction,
Logger: logger,
pendingMessagesWG: pendingMessagesWG,
Expand Down Expand Up @@ -238,10 +235,6 @@ func (g *GCMMessageHandler) handleGCMResponse(cm gcm.CCSMessage) error {
if ccsMessageWithMetadata.Metadata != nil {
ccsMessageWithMetadata.Metadata["deleteToken"] = true
}
handleInvalidToken(
g.InvalidTokenHandlers, cm.From,
parsedTopic.Game, parsedTopic.Platform,
)
case "INVALID_JSON":
l.WithFields(log.Fields{
"category": "JsonError",
Expand Down
16 changes: 0 additions & 16 deletions extensions/gcm_message_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ import (
var _ = Describe("GCM Message Handler", func() {
var feedbackClients []interfaces.FeedbackReporter
var handler *GCMMessageHandler
var invalidTokenHandlers []interfaces.InvalidTokenHandler
var mockClient *mocks.GCMClientMock
var mockDb *mocks.PGMock
var mockKafkaProducerClient *mocks.KafkaProducerClientMock
var mockStatsDClient *mocks.StatsDClientMock
var statsClients []interfaces.StatsReporter
Expand Down Expand Up @@ -72,11 +70,6 @@ var _ = Describe("GCM Message Handler", func() {
statsClients = []interfaces.StatsReporter{c}
feedbackClients = []interfaces.FeedbackReporter{kc}

mockDb = mocks.NewPGMock(0, 1)
it, err := NewTokenPG(config, logger, mockDb)
Expect(err).NotTo(HaveOccurred())
invalidTokenHandlers = []interfaces.InvalidTokenHandler{it}

mockClient = mocks.NewGCMClientMock()
handler, err = NewGCMMessageHandler(
senderID,
Expand All @@ -87,7 +80,6 @@ var _ = Describe("GCM Message Handler", func() {
nil,
statsClients,
feedbackClients,
invalidTokenHandlers,
mockClient,
)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -133,7 +125,6 @@ var _ = Describe("GCM Message Handler", func() {
handler.handleGCMResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
Expect(hook.Entries).To(ContainLogMessage("deleting token"))
})

It("if response has error BAD_REGISTRATION", func() {
Expand All @@ -143,7 +134,6 @@ var _ = Describe("GCM Message Handler", func() {
handler.handleGCMResponse(res)
Expect(handler.responsesReceived).To(Equal(int64(1)))
Expect(handler.failuresReceived).To(Equal(int64(1)))
Expect(hook.Entries).To(ContainLogMessage("deleting token"))
})

It("if response has error INVALID_JSON", func() {
Expand Down Expand Up @@ -507,9 +497,6 @@ var _ = Describe("GCM Message Handler", func() {
feedbackClients = []interfaces.FeedbackReporter{kc}

mockClient = mocks.NewGCMClientMock()
it, err := NewTokenPG(config, logger, mockDb)
Expect(err).NotTo(HaveOccurred())
invalidTokenHandlers = []interfaces.InvalidTokenHandler{it}

handler, err = NewGCMMessageHandler(
senderID,
Expand All @@ -520,7 +507,6 @@ var _ = Describe("GCM Message Handler", func() {
nil,
statsClients,
feedbackClients,
invalidTokenHandlers,
mockClient,
)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -710,7 +696,6 @@ var _ = Describe("GCM Message Handler", func() {
statsClients,
feedbackClients,
nil,
nil,
)
Expect(err).NotTo(HaveOccurred())

Expand All @@ -730,7 +715,6 @@ var _ = Describe("GCM Message Handler", func() {
statsClients,
feedbackClients,
nil,
nil,
)
Expect(handler).To(BeNil())
Expect(err).To(HaveOccurred())
Expand Down

0 comments on commit a81fc73

Please sign in to comment.