Skip to content

Commit ca89e26

Browse files
kylebrowningkylebrowning
authored
Make URLSession client honor the HTTP status code (#241)
`APNSURLSessionClient.send` decided success vs. failure purely by whether the response body decoded as an `APNSErrorResponse`, ignoring the HTTP status code, and threw `urlResponseNotFound` whenever the `apns-id` header was absent. As a result a non-2xx response with an empty/non-JSON body was reported as success, and an error response without `apns-id` surfaced as `urlResponseNotFound` instead of a typed `APNSError`. - Treat `statusCode == 200` as success (per Apple's APNs spec); decode the error body on any other status, falling back to the status code alone when absent. - Look up response headers via the case-insensitive `value(forHTTPHeaderField:)` and tolerate a missing `apns-id`. - Add the `APNSURLSession` target to the test target (both manifests) and an `APNSURLSessionClientTests` suite covering success, header propagation, and the bad-device-token / missing-topic error paths against `APNSTestServer`.
1 parent 88a430c commit ca89e26

4 files changed

Lines changed: 153 additions & 19 deletions

File tree

‎Package.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ let package = Package(
3939
dependencies: [
4040
.target(name: "APNSCore"),
4141
.target(name: "APNS"),
42+
.target(name: "APNSURLSession"),
4243
.target(name: "APNSTestServer"),
4344
.product(name: "Crypto", package: "swift-crypto"),
4445
.product(name: "NIOPosix", package: "swift-nio"),

‎Package@swift-5.10.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ let package = Package(
3939
dependencies: [
4040
.target(name: "APNSCore"),
4141
.target(name: "APNS"),
42+
.target(name: "APNSURLSession"),
4243
.target(name: "APNSTestServer"),
4344
.product(name: "Crypto", package: "swift-crypto"),
4445
.product(name: "NIOPosix", package: "swift-nio"),

‎Sources/APNSURLSession/APNSUrlSessionClient.swift‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,30 +37,32 @@ public struct APNSURLSessionClient: APNSClientProtocol {
3737

3838
/// Make request
3939
let (data, response) = try await URLSession.shared.data(for: urlRequest)
40-
40+
4141
/// Unwrap response
42-
guard let response = response as? HTTPURLResponse,
43-
let apnsIDString = response.allHeaderFields["apns-id"] as? String else {
42+
guard let response = response as? HTTPURLResponse else {
4443
throw APNSUrlSessionClientError.urlResponseNotFound
4544
}
46-
47-
let apnsID = UUID(uuidString: apnsIDString)
48-
let apnsUniqueID = (response.allHeaderFields["apns-unique-id"] as? String).flatMap { UUID(uuidString: $0) }
49-
50-
/// Detect an error
51-
if let errorResponse = try? decoder.decode(APNSErrorResponse.self, from: data) {
52-
let error = APNSError(
53-
responseStatus: response.statusCode,
54-
apnsID: apnsID,
55-
apnsUniqueID: apnsUniqueID,
56-
apnsResponse: errorResponse,
57-
timestamp: errorResponse.timestampInSeconds.flatMap { Date(timeIntervalSince1970: $0) }
58-
)
59-
throw error
60-
} else {
61-
/// Return APNSResponse
45+
46+
/// `value(forHTTPHeaderField:)` performs a case-insensitive lookup, and the
47+
/// `apns-id` header may be absent (e.g. on some error responses), so it is optional.
48+
let apnsID = response.value(forHTTPHeaderField: "apns-id").flatMap { UUID(uuidString: $0) }
49+
let apnsUniqueID = response.value(forHTTPHeaderField: "apns-unique-id").flatMap { UUID(uuidString: $0) }
50+
51+
/// Success/failure is determined by the HTTP status code, per Apple's APNs spec:
52+
/// a `200` is a successful delivery; anything else carries an error reason in the body.
53+
if response.statusCode == 200 {
6254
return APNSResponse(apnsID: apnsID, apnsUniqueID: apnsUniqueID)
6355
}
56+
57+
/// Non-200: decode the error body when present, otherwise surface the status code alone.
58+
let errorResponse = try? decoder.decode(APNSErrorResponse.self, from: data)
59+
throw APNSError(
60+
responseStatus: response.statusCode,
61+
apnsID: apnsID,
62+
apnsUniqueID: apnsUniqueID,
63+
apnsResponse: errorResponse,
64+
timestamp: errorResponse?.timestampInSeconds.flatMap { Date(timeIntervalSince1970: $0) }
65+
)
6466
}
6567

6668
public func shutdown() async throws {

‎Tests/APNSTests/APNSURLSessionClientTests.swift‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the APNSwift open source project
4+
//
5+
// Copyright (c) 2024 the APNSwift project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of APNSwift project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
16+
import APNSCore
17+
import APNSTestServer
18+
@testable import APNSURLSession
19+
import Crypto
20+
import XCTest
21+
22+
final class APNSURLSessionClientTests: XCTestCase {
23+
var server: APNSTestServer!
24+
var client: APNSURLSessionClient!
25+
26+
override func setUp() async throws {
27+
try await super.setUp()
28+
29+
server = APNSTestServer()
30+
try await server.start(port: 0)
31+
32+
client = APNSURLSessionClient(
33+
configuration: .init(
34+
environment: .custom(url: "http://127.0.0.1", port: server.port),
35+
privateKey: try P256.Signing.PrivateKey(pemRepresentation: Self.jwtPrivateKey),
36+
keyIdentifier: "MY_KEY_ID",
37+
teamIdentifier: "MY_TEAM_ID"
38+
)
39+
)
40+
}
41+
42+
override func tearDown() async throws {
43+
try await server?.shutdown()
44+
server = nil
45+
client = nil
46+
try await super.tearDown()
47+
}
48+
49+
func testSendAlert_success() async throws {
50+
let response = try await client.sendAlertNotification(
51+
Self.makeAlert(),
52+
deviceToken: Self.validDeviceToken
53+
)
54+
55+
// A 200 must be treated as success even though the body is `{}`.
56+
XCTAssertNotNil(response.apnsID)
57+
}
58+
59+
func testSendAlert_propagatesHeadersToServer() async throws {
60+
_ = try await client.sendAlertNotification(
61+
Self.makeAlert(),
62+
deviceToken: Self.validDeviceToken
63+
)
64+
65+
let sent = try XCTUnwrap(server.getSentNotifications().first)
66+
XCTAssertEqual(sent.deviceToken, Self.validDeviceToken)
67+
XCTAssertEqual(sent.pushType, "alert")
68+
XCTAssertEqual(sent.topic, "com.example.app")
69+
}
70+
71+
func testSendAlert_badDeviceTokenThrowsTypedError() async throws {
72+
do {
73+
_ = try await client.sendAlertNotification(
74+
Self.makeAlert(),
75+
deviceToken: "not-a-valid-token"
76+
)
77+
XCTFail("Expected an APNSError to be thrown")
78+
} catch let error as APNSError {
79+
// The status code must drive the failure (previously the code keyed off
80+
// whether the body decoded as an error, ignoring the HTTP status).
81+
XCTAssertEqual(error.responseStatus, 400)
82+
XCTAssertEqual(error.reason, .badDeviceToken)
83+
}
84+
}
85+
86+
func testSendAlert_missingTopicThrowsTypedError() async throws {
87+
// Build the request with no topic so the `apns-topic` header is omitted entirely.
88+
let request = APNSRequest(
89+
message: Self.makeAlert(),
90+
deviceToken: Self.validDeviceToken,
91+
pushType: .alert,
92+
expiration: nil,
93+
priority: nil,
94+
apnsID: nil,
95+
topic: nil,
96+
collapseID: nil
97+
)
98+
do {
99+
_ = try await client.send(request)
100+
XCTFail("Expected an APNSError to be thrown")
101+
} catch let error as APNSError {
102+
XCTAssertEqual(error.responseStatus, 400)
103+
XCTAssertEqual(error.reason, .missingTopic)
104+
}
105+
}
106+
107+
// MARK: - Helpers
108+
109+
private static let validDeviceToken = String(repeating: "a", count: 64)
110+
111+
private static func makeAlert() -> APNSAlertNotification<EmptyPayload> {
112+
APNSAlertNotification(
113+
alert: .init(title: .raw("title")),
114+
expiration: .immediately,
115+
priority: .immediately,
116+
topic: "com.example.app",
117+
payload: EmptyPayload()
118+
)
119+
}
120+
121+
private static let jwtPrivateKey = """
122+
-----BEGIN PRIVATE KEY-----
123+
MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQg2sD+kukkA8GZUpmm
124+
jRa4fJ9Xa/JnIG4Hpi7tNO66+OGgCgYIKoZIzj0DAQehRANCAATZp0yt0btpR9kf
125+
ntp4oUUzTV0+eTELXxJxFvhnqmgwGAm1iVW132XLrdRG/ntlbQ1yzUuJkHtYBNve
126+
y+77Vzsd
127+
-----END PRIVATE KEY-----
128+
"""
129+
}
130+
#endif

0 commit comments

Comments
 (0)