Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not sending FailedNotif if identifier not specified. #19

Open
nathany opened this issue Dec 17, 2014 · 5 comments
Open

Not sending FailedNotif if identifier not specified. #19

nathany opened this issue Dec 17, 2014 · 5 comments
Labels

Comments

@nathany
Copy link
Contributor

nathany commented Dec 17, 2014

This relates to #10, as I try to build a synchronous API around Send and FailedNotif.

To get an error from Apple, I am changing a single digit in an otherwise valid device token. I've added logging to timehop/apns.

On this push (with invalid data) nothing comes back from Apple:

2014/12/17 11:32:26 Apple gateway at gateway.sandbox.push.apple.com:2195
2014/12/17 11:32:28 pushing payload...
2014/12/17 11:32:28 json: {"aps":{"alert":{"body":"hello robots"}}}
2014/12/17 11:32:29 Timed out waiting for errors (FailedNotifs). Must be good.

That's not great, but okay. On this push, errrors are coming back from Apple, but the identifiers don't match up so FailedNotifs are never sent.

2014/12/17 11:32:31 pushing payload...
2014/12/17 11:32:31 json: {"aps":{"alert":{"body":"hello robots"}}}
2014/12/17 11:32:31 handleError n.Identifier=0 err.Identifier=2 err=&apns.Error{Command:0x8, Status:0x8, Identifier:0x2, ErrStr:"Invalid token"}
2014/12/17 11:32:31 handleError n.Identifier=0 err.Identifier=2 err=&apns.Error{Command:0x8, Status:0x8, Identifier:0x2, ErrStr:"Invalid token"}
2014/12/17 11:32:32 Timed out waiting for errors (FailedNotifs). Must be good.

If Identifier is set, then I do see an identifier match and the error is sent via FailedNotif:

2014/12/17 11:35:02 pushing payload...
2014/12/17 11:35:02 json: {"aps":{"alert":{"body":"hello robots"}}}
2014/12/17 11:35:02 handleError n.Identifier=1234 err.Identifier=1234 err=&apns.Error{Command:0x8, Status:0x8, Identifier:0x4d2, ErrStr:"Invalid token"}
2014/12/17 11:35:02 reportFailedPush apns.Notification{ID:"user:timestamp", DeviceToken:"30c4afb...", Identifier:0x4d2, Expiration:(*time.Time)(0xc2080bc018), Priority:10, Payload:(*apns.Payload)(0xc2082488c0)} true
2014/12/17 11:35:02 Notif user:timestamp failed with {8 8 1234 Invalid token}
@taylortrimble
Copy link
Contributor

Gross. I expect this to get cleared up with #17 (synchronous Send, no requeueing?), but it's good to keep an eye on it.

@nathany
Copy link
Contributor Author

nathany commented Dec 17, 2014

I'd be happy just to remove the "set an identifier if not set" logic and always require it from the caller.

@nathany
Copy link
Contributor Author

nathany commented Apr 23, 2015

I still think removing determineIdentifier entirely would be the best bet. Just require the identifier from the client.

@nathany nathany added the bug label Apr 23, 2015
@taylortrimble
Copy link
Contributor

I do like the idea of "transparent defaults", but I do see your point as well. I wouldn't be heartbroken either way. 😉

@nathany nathany mentioned this issue Apr 30, 2015
11 tasks
@nathany
Copy link
Contributor Author

nathany commented May 20, 2015

When replaying notifications, should it use the same IDs or call determineIdentifier for new ones?

nathany added a commit that referenced this issue May 20, 2015
at least for now, may bring this functionality back

ref #19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants