feat: Add experimental native APNs API#547
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #547 +/- ##
===========================================
- Coverage 100.00% 99.18% -0.82%
===========================================
Files 8 11 +3
Lines 1172 1841 +669
===========================================
+ Hits 1172 1826 +654
- Misses 0 15 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a native APNS implementation using HTTP/2 and token-based ES256 authentication: new APNSToken (JWT generation/caching), APNSConnection (HTTP/2 session, retries, timeouts), APNSNative (provider orchestration, routing, retries), README docs, adapter integration, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Adapter as ParsePushAdapter
participant Native as APNSNative
participant Provider as Provider
participant Token as APNSToken
participant Conn as APNSConnection
participant APNS as Apple APNS
Client->>Adapter: send(data, devices)
Adapter->>Native: send(data, devices)
Native->>Native: group devices by appIdentifier
Native->>Native: select providers by topic
Native->>Native: _generateNotification(data, headers)
Native->>Provider: sendThroughProvider(notification, devices)
loop per-device
Provider->>Token: token.current
alt token expired
Token->>Token: regenerate JWT
end
Provider->>Conn: send(deviceToken, headers, payload, authToken)
Conn->>APNS: POST /3/device/{deviceToken} + Bearer
APNS-->>Conn: HTTP/2 response
alt 200
Conn->>Provider: success
else 403 + ExpiredProviderToken
Provider->>Token: refresh()
Token->>Token: regenerate JWT
Provider->>Conn: resend with new token
else 5xx/408/429
Conn->>Conn: retry with backoff
else timeout (status 0)
Conn->>Conn: cancel stream, return RequestTimeout
end
end
Provider->>Native: return {sent, failed}
Native->>Adapter: aggregated results
Adapter->>Client: return results
sequenceDiagram
participant Conn as APNSConnection
participant Session as HTTP/2 Session
participant Handler as Request Handler
Conn->>Conn: first send() -> _getSession()
Conn->>Session: http2.connect(endpoint)
Session-->>Conn: connected
Conn->>Session: start periodic ping (unref)
Conn->>Handler: create POST request + write body
Session-->>Handler: response chunks
Handler->>Handler: aggregate & parse response
Handler->>Conn: resolve {status, body}
Note over Session,Conn: goaway -> mark draining, next send() creates new session
Note over Conn: destroy() stops ping, destroys session, _session = null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
spec/APNSToken.spec.js (1)
11-22: Consider adding ESLint environment directive for test globals.The static analysis warns that
beforeAllandafterAllare not defined. These are valid Jest/Jasmine globals, but ESLint may not recognize them without proper configuration.🔧 Optional: Add ESLint directive at top of file
+/* eslint-env jest */ import { generateKeyPairSync } from 'node:crypto'; import { writeFileSync, mkdtempSync, rmSync } from 'node:fs';Alternatively, ensure
jestis configured in the ESLint environment for spec files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/APNSToken.spec.js` around lines 11 - 22, ESLint is flagging the Jest globals beforeAll and afterAll as undefined; add an ESLint environment directive or enable the jest environment for this spec so those globals are recognized. Specifically, add the eslint-env jest directive at the top of this test file or update your ESLint config to include "jest" for spec files so functions like beforeAll, afterAll (and other Jest globals) are treated as defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/APNSNative.spec.js`:
- Around line 451-458: The error message string "Unkown status" contains a typo;
search for that exact literal in APNSNative._handlePushFailure and
APNS._handlePushFailure and update it to "Unknown status" in both implementation
files, and update any tests (e.g., the assertion in APNSNative.spec.js) that
expect the misspelled string so they assert "Unknown status" instead; run tests
to confirm the fix.
In `@src/APNSConnection.js`:
- Around line 46-55: The callbacks in _request and other handlers are tearing
down this._session unconditionally; change them to only teardown/clear the
connection if the session instance that failed matches the active session
(compare the closed/failing session variable to this._session before calling
this._destroySession() or setting this._session = null). Locate uses in _request
(the try/catch around session.request), the request/error/ping callbacks and
ensure each path checks if (this._session === session) before destroying or
clearing; otherwise only handle retry/reject for that specific session without
touching the current active session.
In `@src/APNSNative.js`:
- Around line 133-139: The code buckets devices by appIdentifier into a plain
object (devicesPerAppIdentifier) using untrusted keys (appIdentifier), which can
collide with prototype properties like __proto__ and break .push(); fix this by
replacing the plain object with a safe map-style container (e.g., use a Map or
an object created via Object.create(null)) and update the grouping logic that
iterates allDevices and accesses appIdentifier so it uses the safe container’s
get/set or safe own-property checks when creating and pushing arrays for each
key (update references to devicesPerAppIdentifier and the forEach grouping code
accordingly).
- Around line 341-346: The code currently treats failure.status falsily so
status === 0 (used by APNSConnection for timeouts/network errors) is lost;
update the conditional in APNSNative where it checks failure.status to use an
explicit undefined check (e.g., failure.status !== undefined) so status 0 is
preserved, keep checking failure.response && failure.response.reason, and call
APNSNative._createErrorPromise(failure.device, failure.response.reason) as
before; also fix the typo 'Unkown' to 'Unknown' in the fallback log/message so
the unknown-status branch reads clearly.
---
Nitpick comments:
In `@spec/APNSToken.spec.js`:
- Around line 11-22: ESLint is flagging the Jest globals beforeAll and afterAll
as undefined; add an ESLint environment directive or enable the jest environment
for this spec so those globals are recognized. Specifically, add the eslint-env
jest directive at the top of this test file or update your ESLint config to
include "jest" for spec files so functions like beforeAll, afterAll (and other
Jest globals) are treated as defined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5a5de8a-6b4f-4d66-b1f9-0e935a37b21d
📒 Files selected for processing (10)
README.mdspec/APNSConnection.spec.jsspec/APNSNative.spec.jsspec/APNSToken.spec.jsspec/ParsePushAdapter.spec.jssrc/APNSConnection.jssrc/APNSNative.jssrc/APNSToken.jssrc/ParsePushAdapter.jssrc/index.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/APNSNative.js`:
- Around line 38-42: The migration shim currently unconditionally copies
providerArgs.bundleId into providerArgs.topic; change it so the deprecated alias
does not override an explicit topic by only backfilling when providerArgs.topic
is null/undefined. In the block that references providerArgs, bundleId and
LOG_PREFIX, check if providerArgs.topic is absent (e.g., === undefined or ==
null) before assigning providerArgs.topic = providerArgs.bundleId, and emit the
deprecation log only when you actually use bundleId to set topic.
- Around line 127-130: Replace falsy-driven `||` checks with nullish coalescing
(`??`) for header fields (expirationTime, collapseId, pushType, priority) and in
the header-building logic so legitimate values like 0 are preserved; update all
presence checks to use `!= null` rather than truthiness. In
_generateNotification (the switch/filter that builds the APNs payload) add the
snake_case variants expiration_time, collapse_id, and push_type to the cases
that are excluded from the payload (they should remain only in headers). Ensure
the header construction code that reads those fields uses `??` and `!= null`
checks consistently so snake_case inputs don’t leak into the final notification
body.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7c7ba5b-d966-4eac-b62c-55b48df1398f
📒 Files selected for processing (6)
spec/APNS.spec.jsspec/APNSConnection.spec.jsspec/APNSNative.spec.jssrc/APNS.jssrc/APNSConnection.jssrc/APNSNative.js
✅ Files skipped from review due to trivial changes (2)
- src/APNS.js
- spec/APNS.spec.js
# [8.4.0](8.3.1...8.4.0) (2026-03-31) ### Features * Add experimental native APNs API ([#547](#547)) ([cabcd54](cabcd54))
|
🎉 This change has been released in version 8.4.0 |
Feature
Adds an opt-in native APNs HTTP/2 client as an alternative to
@parse/node-apn. Uses only built-in Node.js modules (node:http2,node:crypto) with zero additional dependencies.Enable with
useNativeAPNs: truein the push config. Only supports token-based auth (.p8). The existing@parse/node-apnpath remains the default and is unchanged.Tasks
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests