From ab6d25e34751ce4f926b0e6fd9d9eeab74ac8c7c Mon Sep 17 00:00:00 2001 From: Anas Date: Sat, 2 May 2026 21:48:06 +0100 Subject: [PATCH 1/3] Guard handleError against non-string error.code incomingError.code can be a non-string (e.g. when an upstream layer fails before FCM is reached and surfaces a numeric HTTP status), in which case .startsWith throws TypeError inside the error handler and masks the original error. The 5xx response body became an unhelpful stack trace from the handler itself, making the real problem hard to trace from the calling app's logs. Adding a typeof guard keeps the messaging-error fast path while letting non-FCM errors fall through to the generic reportError + InternalError response, which preserves the actual cause. Reproducible by sending a request that triggers any non-FCM error in sendNotification (e.g. a transient network failure surfaced with a numeric code). With the guard, the handler returns the InternalError JSON; without it, the request 500s with a TypeError. --- functions/handlers.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 From 981e5d2f39028671f8e9a393f7ba83aeffbb36c0 Mon Sep 17 00:00:00 2001 From: Anas Date: Sat, 2 May 2026 21:48:46 +0100 Subject: [PATCH 2/3] Stringify object/array data values as JSON instead of "[object Object]" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FCM data fields must be strings, so the proxy coerces every value with String(). For objects and arrays this produces "[object Object]" or "[object Object],[object Object]" which the receiving app then can't decode. This bites whenever the calling integration's template renderer auto-parses a JSON-shaped string back into a native list/object and sends it as structured JSON in the request body. The proxy receives the parsed value and stringifies it as JS rather than as JSON. Switch to JSON.stringify for non-null objects/arrays. Strings, numbers, and booleans still go through String() unchanged. Reproducible from Home Assistant by sending notify.mobile_app_* with a samples list inside the data field that originates from a Jinja template — the template renders to a JSON-array string, HA parses it back into a Python list before serializing the request body, and the device-side parser receives "[object Object],[object Object]". --- functions/android.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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); } }); } From 3aee67c7f4d5a4cdd2b3b1a87c25b500b5e9aa70 Mon Sep 17 00:00:00 2001 From: Anas Date: Sat, 2 May 2026 22:15:38 +0100 Subject: [PATCH 3/3] Add regression tests for both bugfixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback. Covers: - handleError with non-string error.code (numeric, undefined) — repros the original TypeError-on-.startsWith crash, asserts the proxy still responds 500 instead of hanging. - android.js whitelist serialization — array values produce a JSON string the receiver can parse, object values likewise, and null falls through to the primitive String() branch unchanged. --- functions/test/android.test.js | 50 +++++++++++++++++++++++++++++++ functions/test/fcm-errors.test.js | 46 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 functions/test/android.test.js 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', + }); + }); });