fix handleError on non-string error.code and serialize object/array data values as json#294
Open
anasmadrhar wants to merge 3 commits into
Open
Conversation
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.
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]".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses two small runtime bugs in the Firebase push proxy: one in centralized error handling for non-FCM failures, and one in Android payload construction where structured data values were being flattened into unusable strings before being sent to FCM.
Changes:
- Guard
handleErroragainst non-stringincomingError.codevalues before calling.startsWith(...). - Serialize Android
datavalues withJSON.stringifywhen they are arrays/objects, while preservingString(...)coercion for primitives.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
functions/handlers.js |
Hardens Firebase messaging error classification so non-string error codes do not crash error handling. |
functions/android.js |
Changes Android payload data coercion so object/array values are preserved as JSON strings instead of lossy stringification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `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/')) { |
Comment on lines
+116
to
+121
| 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); |
This was referenced May 2, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
two small bugs i hit while running this proxy on my own gcp project to handle pushes for an app fork.
1. handleError crashes when error.code isn't a string
in
handlers.js, whenmessaging.sendfails before reaching fcm (e.g. dns/network issue, or any non-fcm error path), the error doesn't always have a stringcode. it can be a number or undefined. the existing check goes straight toincomingError.code.startsWith('messaging/')which throwsTypeError: incomingError.code.startsWith is not a function, and that throw escapes out ofhandleErroritself — so the original error is masked and the response handler never returns. clients see a hung request / 500 with no useful info in logs.guarded with
typeof incomingError.code === 'string'before the.startsWith. behavior for realmessaging/*codes is unchanged.2. object/array data values become the literal string
[object Object]in
android.js, theandroidNotificationKeysloop doesString(req.body.data[key])to coerce each value to a string (fcm data values must be strings). that's fine for primitives but for arrays/objects it produces"[object Object]"or"a,b,c", neither of which the receiving app can decode.this happens in practice when the calling integration's template engine auto-parses a json-string back into a native list/dict before posting. the sender thinks they sent
[1,2,3], the proxy ships"1,2,3", the app can'tJSON.parseit.switched to
JSON.stringify(v)whenvis a non-null object, plainString(v)otherwise. no behavior change for primitive values.both changes are independent and tiny, happy to split into two prs if preferred.