Skip to content
This repository has been archived by the owner on May 27, 2020. It is now read-only.

ServiceStatus success #57

Closed
mludi opened this issue Aug 10, 2017 · 2 comments
Closed

ServiceStatus success #57

mludi opened this issue Aug 10, 2017 · 2 comments

Comments

@mludi
Copy link

mludi commented Aug 10, 2017

Hi @matthijs2704 ,

there is an issue with the version 2 of this library.

If you send a notification to a non exsiting or bad token, it will return success in the result

vaporAPNS.send(pushMessage, to: deviceTokens) { result in
   if case let .success(_, deviceToken, serviceStatus) = result, case .success = serviceStatus {
        print("💚 success: \(deviceToken) and status \(serviceStatus)")
    }
    else if case let .error(_, deviceToken, error) = result {
        print("❤️ error: \(deviceToken) with \(error)")
    }
    else if case let .networkError(error) = result {
        print("❤️ networkError: \(error)")
    }
}            

so result contains:

▿ Result
  ▿ success : 2 elements
    - apnsId : "some random number"
    - deviceToken : "a valid token, really ;)"
    - serviceStatus : VaporAPNS.ServiceStatus.success


(lldb) po result
▿ Result
  ▿ success : 2 elements
    - apnsId : "some random number"
    - deviceToken : "asdf"
    - serviceStatus : VaporAPNS.ServiceStatus.success
@Cellane
Copy link

Cellane commented Aug 26, 2017

Possibly related: I think there's something seriously wrong with reporting the result. Few days back, I was debugging push notifications suddenly not getting delivered even though I was getting the .success result. Turns out they were never accepted by APNS servers due to a typo that occurred in the private or public key, can't remember which one. At any rate, neither this case or the one originally reported definitely shouldn't end up with a .success result case.

For the record, I'm using the new HTTP/2 method of connecting to APNS (just with private and public key manually extracted from the p8 file, to be able to supply it as two environmental variables), if that changes anything.

@akeskiner
Copy link

akeskiner commented Oct 3, 2017

I just found the cause of the bug, will try to fix it when I have time.

if responseData != "" {
    // Get JSON from loaded data string
    let jsonNode = JSON(.bytes(responseData.makeBytes()), in: nil).makeNode(in: nil)
    if let reason = jsonNode["reason"]?.string {
        result = Result.error(apnsId: message.messageId, deviceToken: deviceToken, error: APNSError.init(errorReason: reason))
    } else {
        result = Result.success(apnsId: message.messageId, deviceToken: deviceToken, serviceStatus: .success)
    }
} else {
    result = Result.success(apnsId: message.messageId, deviceToken: deviceToken, serviceStatus: .success)
}

Here, the response can't be parsed and jsonNode["reason"] is nil, so the result is always success.

Edit: Just realized that #50 fixes the problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants