diff --git a/functions/android.js b/functions/android.js index 160f511..6d7ed56 100644 --- a/functions/android.js +++ b/functions/android.js @@ -113,7 +113,12 @@ module.exports = { androidNotificationKeys.forEach((key) => { if (Object.hasOwn(req.body.data, key)) { - payload.data[key] = String(req.body.data[key]); + const v = req.body.data[key]; + // FCM data values must be strings. For arrays/objects (e.g. when the + // calling integration's template engine auto-parses a JSON-string + // back into a list), String(value) produces "[object Object]" which + // the receiving app then can't decode as JSON. Stringify properly. + payload.data[key] = (v !== null && typeof v === 'object') ? JSON.stringify(v) : String(v); } }); } diff --git a/functions/handlers.js b/functions/handlers.js index adf4677..5dc50ae 100644 --- a/functions/handlers.js +++ b/functions/handlers.js @@ -168,8 +168,11 @@ function handleError(req, res, payload = {}, step, incomingError, shouldExit = t incomingError = new Error(incomingError); } - // Handle Firebase Messaging errors with appropriate status codes - if (incomingError.code?.startsWith('messaging/')) { + // Handle Firebase Messaging errors with appropriate status codes. + // `code` can be a non-string (e.g. numeric HTTP status from a network failure + // before reaching FCM); without the type guard, .startsWith throws and masks + // the underlying error. + if (typeof incomingError.code === 'string' && incomingError.code.startsWith('messaging/')) { const errorCode = incomingError.code.replace('messaging/', ''); // For specific token errors, skip reporting and return immediately diff --git a/functions/test/android.test.js b/functions/test/android.test.js new file mode 100644 index 0000000..577b536 --- /dev/null +++ b/functions/test/android.test.js @@ -0,0 +1,50 @@ +'use strict'; + +const android = require('../android.js'); + +describe('android.js createPayload', () => { + function makeReq(data) { + return { + body: { + data, + registration_info: {}, + }, + }; + } + + describe('whitelisted data values', () => { + test('primitive values are stringified', () => { + const { payload } = android.createPayload(makeReq({ tag: 'foo', importance: 4 })); + expect(payload.data.tag).toBe('foo'); + expect(payload.data.importance).toBe('4'); + }); + + test('array values are JSON-stringified, not coerced via String()', () => { + // Repro: HA's template engine auto-parses a JSON array string back into a + // native list before posting. Without JSON.stringify, the proxy ships + // "1,2,3" (Array.prototype.toString), and the receiving app's JSON.parse + // throws SyntaxError. Object array would arrive as "[object Object]". + const samples = [ + { time: '2026-01-01T08:00:00Z', value: 60 }, + { time: '2026-01-01T08:00:30Z', value: 64 }, + ]; + const { payload } = android.createPayload(makeReq({ vibrationPattern: samples })); + expect(payload.data.vibrationPattern).toBe(JSON.stringify(samples)); + expect(JSON.parse(payload.data.vibrationPattern)).toEqual(samples); + }); + + test('object values are JSON-stringified', () => { + const obj = { a: 1, b: 'two' }; + const { payload } = android.createPayload(makeReq({ intent_extras: obj })); + expect(payload.data.intent_extras).toBe(JSON.stringify(obj)); + }); + + test('null values fall through to String() and become "null"', () => { + // Documents existing behavior — JSON.stringify(null) is also 'null', but + // explicitly guarding `typeof v === 'object' && v !== null` keeps null + // out of the JSON branch so this stays consistent with prior versions. + const { payload } = android.createPayload(makeReq({ tag: null })); + expect(payload.data.tag).toBe('null'); + }); + }); +}); diff --git a/functions/test/fcm-errors.test.js b/functions/test/fcm-errors.test.js index 1a3a9d7..71490a3 100644 --- a/functions/test/fcm-errors.test.js +++ b/functions/test/fcm-errors.test.js @@ -196,4 +196,50 @@ describe('FCM Error Handling', () => { expect(mockLogging.log).toHaveBeenCalledWith('errors-sendNotification'); expect(mockLogInstance.write).toHaveBeenCalled(); }); + + test('should not crash when error.code is a non-string (numeric HTTP status)', async () => { + // Repro: a network failure before reaching FCM produces an Error with a numeric + // `code` (e.g. 503). Without the type guard in handleError, calling + // .startsWith('messaging/') on the number throws a TypeError that escapes + // handleError itself and the response is never sent. + const error = new Error('Service unavailable'); + error.code = 503; + mockMessaging.send.mockRejectedValue(error); + + const mockLogInstance = { + write: jest.fn((entry, callback) => callback()), + entry: jest.fn(() => ({})), + }; + mockLogging.log.mockReturnValue(mockLogInstance); + + await indexModule.handleRequest(req, res, payloadHandler); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.send).toHaveBeenCalledWith({ + errorType: 'InternalError', + errorStep: 'sendNotification', + message: 'Service unavailable', + }); + expect(mockLogInstance.write).toHaveBeenCalled(); + }); + + test('should not crash when error.code is undefined', async () => { + const error = new Error('Generic failure with no code'); + mockMessaging.send.mockRejectedValue(error); + + const mockLogInstance = { + write: jest.fn((entry, callback) => callback()), + entry: jest.fn(() => ({})), + }; + mockLogging.log.mockReturnValue(mockLogInstance); + + await indexModule.handleRequest(req, res, payloadHandler); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.send).toHaveBeenCalledWith({ + errorType: 'InternalError', + errorStep: 'sendNotification', + message: 'Generic failure with no code', + }); + }); });