librato / librato-services

Services for Librato Metrics
MIT License
14 stars 24 forks source link

Hotfix/sns-sms #134

Closed chancefeick closed 8 years ago

chancefeick commented 8 years ago

Hotfix for SNS topics not delivering to SMS subscriptions. Result of SNS backend (silently?) failing when hitting SMS length limits from large JSON payloads. The following protocols are supported:

{
  "default": "foobar", 
  "email": "foobar", 
  "sqs": "foobar", 
  "lambda": "foobar", 
  "http": "foobar", 
  "https": "foobar", 
  "sms": "foobar", 
  "APNS": "{\"aps\":{\"alert\": \"foobar\"} }", 
  "APNS_SANDBOX":"{\"aps\":{\"alert\":\"foobar\"}}", 
  "APNS_VOIP":"{\"aps\":{\"alert\":\"foobar\"}}", 
  "APNS_VOIP_SANDBOX": "{\"aps\":{\"alert\": \"foobar\"} }", 
  "MACOS":"{\"aps\":{\"alert\":\"foobar\"}}", 
  "MACOS_SANDBOX": "{\"aps\":{\"alert\": \"foobar\"} }", 
  "GCM": "{ \"data\": { \"message\": \"foobar\" } }", 
  "ADM": "{ \"data\": { \"message\": \"foobar\" } }", 
  "BAIDU": "{\"title\":\"foobar\",\"description\":\"foobar\"}", 
  "MPNS" : "<?xml version=\"1.0\" encoding=\"utf-8\"?><wp:Notification xmlns:wp=\"WPNotification\"><wp:Tile><wp:Count>ENTER COUNT</wp:Count><wp:Title>foobar</wp:Title></wp:Tile></wp:Notification>", 
  "WNS" : "<badge version\"1\" value\"23\"/>"
}

:message_structure allows sending a different message for each protocol. We're only setting a specific message for SMS, then defaulting to the previous payload for the remaining protocols. Would've been nice to use #html for "email", but SNS unfortunately only renders plain/text.

screenshot

@miketang415

akahn commented 8 years ago

So how does this handle messages that are over the SMS length limit? I can't quite tell from the code.

chancefeick commented 8 years ago

@akahn addressed truncating in 5641d021c7b54b5c327811e634a86465db503d71

akahn commented 8 years ago

Looking good. I say :shipit: