Skip to content

Commit 627989f

Browse files
committed
Refactor proxy support to match Firebase Admin SDK capabilities
Based on research and feedback from issue #220: - Remove httpsAgent support (Firebase only supports httpAgent in AppOptions) - Pass httpAgent to credential factory for proper authentication proxy support - Single httpAgent parameter handles both HTTP and HTTPS connections - Update documentation to clarify httpAgent usage with HttpsProxyAgent for HTTPS proxies - Update tests to reflect httpAgent-only configuration - Maintain legacyHttpTransport support for HTTP/1.1 transport All 88 tests passing (87 existing + 1 legacyHttpTransport test) Simplify credential assignment using || operator Replace if/else block with more concise || operator for getting or creating credential with httpAgent support. Maintains same functionality while improving readability.
1 parent 894bee1 commit 627989f

File tree

4 files changed

+15
-54
lines changed

4 files changed

+15
-54
lines changed

.github/copilot-instructions.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ This repository implements a Node.js module for sending push notifications acros
6868
- `storageBucket` (optional) - Cloud Storage bucket
6969
- `serviceAccountId` (optional) - Service account email
7070
- `databaseAuthVariableOverride` (optional) - Auth override for RTDB rules
71-
- `httpAgent` (optional) - HTTP proxy agent for network requests
72-
- `httpsAgent` (optional) - HTTPS proxy agent for network requests
73-
- `legacyHttpTransport` (optional) - Enable HTTP/1.1 transport instead of HTTP/2 (for compatibility with older Node.js or network restrictions)
71+
- `httpAgent` (optional) - HTTP/HTTPS proxy agent for network requests (use HttpProxyAgent for HTTP proxies, HttpsProxyAgent for HTTPS proxies)
72+
- `legacyHttpTransport` (optional) - Enable HTTP/1.1 transport instead of HTTP/2 (for compatibility with older Node.js or network restrictions, required for proper proxy support)
7473

7574
All optional properties are dynamically added to Firebase initialization if defined.
7675

README.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ const settings = {
5959
databaseURL: 'https://your-database.firebaseio.com', // Realtime Database URL (optional)
6060
storageBucket: 'your-bucket.appspot.com', // Cloud Storage bucket (optional)
6161
serviceAccountId: '[email protected]', // Service account email (optional)
62-
httpAgent: undefined, // HTTP Agent for proxy support (optional)
63-
httpsAgent: undefined, // HTTPS Agent for proxy support (optional)
62+
httpAgent: undefined, // HTTP Agent for proxy support - use HttpProxyAgent for HTTP proxies, HttpsProxyAgent for HTTPS proxies (optional)
6463
legacyHttpTransport: false, // Enable HTTP/1.1 instead of HTTP/2 (optional)
6564
},
6665
apn: {
@@ -475,8 +474,7 @@ The following Firebase Admin SDK `AppOptions` are supported and can be passed in
475474
- `storageBucket` - Cloud Storage bucket name (optional)
476475
- `serviceAccountId` - Service account email (optional)
477476
- `databaseAuthVariableOverride` - Auth variable override for Realtime Database (optional)
478-
- `httpAgent` - HTTP Agent for proxy support (optional, see [Proxy](#proxy) section)
479-
- `httpsAgent` - HTTPS Agent for proxy support (optional, see [Proxy](#proxy) section)
477+
- `httpAgent` - HTTP/HTTPS Agent for proxy support (optional, use HttpProxyAgent for HTTP proxies, HttpsProxyAgent for HTTPS proxies, see [Proxy](#proxy) section)
480478
- `legacyHttpTransport` - Enable HTTP/1.1 transport instead of HTTP/2 (optional, for compatibility with older Node.js versions or network restrictions)
481479

482480
```js

src/sendFCM.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,19 @@ const sendChunk = (firebaseApp, recipients, message) => {
6767

6868
const sendFCM = (regIds, data, settings) => {
6969
const appName = `${settings.fcm.appName}`;
70+
71+
// Get or create credential with httpAgent for proper proxy support in authentication
72+
const credential =
73+
settings.fcm.credential ||
74+
firebaseAdmin.credential.cert(settings.fcm.serviceAccountKey, settings.fcm.httpAgent);
75+
7076
const opts = {
71-
credential:
72-
settings.fcm.credential || firebaseAdmin.credential.cert(settings.fcm.serviceAccountKey),
77+
credential,
7378
};
7479

7580
// Add optional Firebase AppOptions properties if provided
7681
const optionalProps = [
7782
'httpAgent',
78-
'httpsAgent',
7983
'projectId',
8084
'databaseURL',
8185
'storageBucket',

test/send/sendFCM.js

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -154,47 +154,8 @@ describe('push-notifications-fcm', () => {
154154
});
155155
});
156156

157-
it('should accept httpsAgent in settings', (done) => {
158-
const mockHttpsAgent = {};
159-
const mockInitializeApp = sinon.stub(firebaseAdmin, 'initializeApp').returns({
160-
messaging: () => ({
161-
sendEachForMulticast: () =>
162-
Promise.resolve({
163-
successCount: 1,
164-
failureCount: 0,
165-
responses: [{ error: null }],
166-
}),
167-
}),
168-
});
169-
170-
const fcmOptsWithProxy = {
171-
fcm: {
172-
name: 'testAppNameProxyHttps',
173-
credential: { getAccessToken: () => Promise.resolve({}) },
174-
httpsAgent: mockHttpsAgent,
175-
},
176-
};
177-
178-
const pnWithProxy = new PN(fcmOptsWithProxy);
179-
180-
pnWithProxy
181-
.send(regIds, message)
182-
.then(() => {
183-
// Verify that initializeApp was called with httpsAgent
184-
const callArgs = mockInitializeApp.getCall(0).args[0];
185-
expect(callArgs.httpsAgent).to.equal(mockHttpsAgent);
186-
mockInitializeApp.restore();
187-
done();
188-
})
189-
.catch((err) => {
190-
mockInitializeApp.restore();
191-
done(err);
192-
});
193-
});
194-
195-
it('should accept both httpAgent and httpsAgent in settings', (done) => {
157+
it('should accept httpAgent in settings for both HTTP and HTTPS', (done) => {
196158
const mockHttpAgent = {};
197-
const mockHttpsAgent = {};
198159
const mockInitializeApp = sinon.stub(firebaseAdmin, 'initializeApp').returns({
199160
messaging: () => ({
200161
sendEachForMulticast: () =>
@@ -208,10 +169,9 @@ describe('push-notifications-fcm', () => {
208169

209170
const fcmOptsWithProxy = {
210171
fcm: {
211-
name: 'testAppNameProxyBoth',
172+
name: 'testAppNameProxyHttpsViaHttpAgent',
212173
credential: { getAccessToken: () => Promise.resolve({}) },
213174
httpAgent: mockHttpAgent,
214-
httpsAgent: mockHttpsAgent,
215175
},
216176
};
217177

@@ -220,10 +180,10 @@ describe('push-notifications-fcm', () => {
220180
pnWithProxy
221181
.send(regIds, message)
222182
.then(() => {
223-
// Verify that initializeApp was called with both agents
183+
// Verify that initializeApp was called with httpAgent
184+
// httpAgent handles both HTTP and HTTPS connections in Firebase Admin SDK
224185
const callArgs = mockInitializeApp.getCall(0).args[0];
225186
expect(callArgs.httpAgent).to.equal(mockHttpAgent);
226-
expect(callArgs.httpsAgent).to.equal(mockHttpsAgent);
227187
mockInitializeApp.restore();
228188
done();
229189
})

0 commit comments

Comments
 (0)