Skip to content

[$175] [Sentry: APP-27B] Pusher chunked JSON parse breaks on truncated payload #102745

Open

Description

@mountiny

Sentry

https://expensify.sentry.io/issues/APP-27B

Impact (snapshot at filing)

  • Users (total since first seen): 20
  • Events: 409
  • Users (last 14d): not pulled separately
  • First seen: 2025-12-07 (~10 months ago, still ongoing)
  • Last seen: now (ongoing)
  • Platform: web
  • App version(s): seen across many releases through current production (9.4.99)
  • Affected route(s) (parameterized): /search/r/:reportID
  • Mechanism: auto.browser.browserapierrors.xhr.onprogress

Stack trace (top frames, first-party only)

pusher-js xhr streaming transport: onChunk
JSON.parse(chunk.data.slice(1) || '[]')

Error: SyntaxError: Unexpected token 'l', "lass FPClassifier {" is not valid JSON

Suspected cause

Pusher's chunked-XHR transport parses an incomplete chunk as JSON when the response looks like a truncated JS class definition (class FPClassifier {}), suggesting the client occasionally receives a different content type than expected mid-stream.

Reproduction

Unknown — see Sentry events linked from the Sentry issue.

Related

  • Sibling Sentry IDs: none identified
  • Prior GH issues: none found
Upwork Automation - Do Not Edit

Activity

  1. self-assigned this
    on Oct 1, 2026
  2. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Oct 1, 2026
  3. melvin-bot commented on Oct 1, 2026

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @abzokhattab (External)

  4. changed the title [-][Sentry: APP-27B] Pusher chunked JSON parse breaks on truncated payload[/-] [+][$175] [Sentry: APP-27B] Pusher chunked JSON parse breaks on truncated payload[/+] on Oct 1, 2026
  5. melvin-bot commented on Oct 1, 2026

    @melvin-bot
  6. MelvinBot commented on Oct 1, 2026

    @MelvinBot
    Contributor

    Proposal

    What is the root cause of that problem?

    Pusher did not send a truncated payload. A complete line of foreign JavaScript (class FPClassifier {) reached pusher-js inside an HTTP 200 response body. pusher-js then tried to parse it as a protocol frame.

    When WebSockets fail or are slow, pusher-js falls back to xhr_streaming. That transport splits the response into lines and uses each line's first character as the frame type. A line that starts with c is treated as a "close" frame and passed to JSON.parse without a try/catch (http_socket.ts:109-111). The parse throws inside the XHR onprogress handler. Sentry wraps that handler, so the error is reported as auto.browser.browserapierrors.xhr.onprogress.

    FPClassifier does not exist in App, its dependencies, or the service worker. The text most likely comes from the user's environment, such as an extension that overrides XMLHttpRequest or a proxy that rewrites responses. That fits the small number of affected users (20).

    What changes do you think we should make in order to solve the problem?

    Add a patch-package patch for pusher-js@8.3.0 in patches/pusher-js/, with a details.md like the other patches.

    1. In HTTPSocket.onChunk, wrap only the JSON.parse call for the o, a, m, and c frames in a try/catch.
    2. If the parse fails, skip the line, the same way the switch already ignores lines with unknown prefixes.
    3. Do not wrap the onOpen, onEvent, or onClose calls. They run App's channel callbacks, and a broad catch would hide real App errors.

    The patch must change dist/web/pusher-with-encryption.js, because App imports pusher-js/with-encryption. This matches how the WebSocket transport already handles bad messages (it catches the error and emits MessageParseError). Upstream pusher-js still has the same unguarded code, so upgrading will not fix this.

    What alternative solutions did you explore? (Optional)

    • Sentry ignoreErrors / beforeSend filter: This hides the report, but the uncaught exception still happens. An /FPClassifier/ filter only matches this one string. The existing denyUrls and third-party filters in setupSentry.ts do not match it, because the throwing frame is pusher-js inside our own bundle.
    • Disable xhr_streaming / xhr_polling: Not recommended. These transports are the only fallback when WebSockets are blocked. xhr_polling also uses the same onChunk code.
    Evidence and open questions
    • App uses the default transport strategy and sets only cluster, activityTimeout, pongTimeout, and the authorizer (src/libs/Pusher/index.ts:88-98).
    • pusher-js only passes complete, newline-terminated lines to onChunk, so the input was not cut off. Non-200 responses are dropped before parsing (http_socket.ts:81-84).
    • The class body lines start with whitespace, so pusher-js ignores them. Only the class … line throws. Injected lines that start with a, o, or m (for example, async …) would throw too and may appear as separate Sentry issues. The proposed patch covers them.
    • Not verified: what injects FPClassifier. The Sentry MCP connection failed during this run, so I could not check the browser or extension tags on the events. Checking those tags would confirm the source. The fix does not depend on it.
    • The /search/r/:reportID route is probably where affected users happen to be. Nothing in the code ties the error to that route.

    Possibly related: Audit Sentry error volume and filter noise to stop paying overages


    Next Steps for Contributor+ team:
    To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/102745) to create a draft PR.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting Melvin's proposal.


    view run

  7. camesenin commented on Oct 1, 2026

    @camesenin

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    Sentry issue APP-27B reports SyntaxError: Unexpected token 'l', "lass FPClassifier {" is not valid JSON thrown from pusher-js's XHR streaming transport (xhr.onprogress). This occurs on web clients when Pusher attempts to use HTTP chunked/streaming fallback transports and receives an unexpected HTML/JS response (such as proxy/firewall injection or bot detection like class FPClassifier) instead of a valid Pusher protocol chunk.


    What is the root cause of that problem?

    In src/libs/Pusher/index.ts:

    const options: Options = {
        cluster: args.cluster,
        activityTimeout: CONST.PUSHER.ACTIVITY_TIMEOUT_MS,
        pongTimeout: CONST.PUSHER.PONG_TIMEOUT_MS,
    };
    1. options does not specify enabledTransports.
    2. By default in pusher-js on web browsers, when enabledTransports is omitted, the library includes HTTP-based fallback transports: ['ws', 'wss', 'xhr_streaming', 'xhr_polling', 'sockjs'].
    3. When a client experiences network fluctuations, corporate firewall restrictions, or captive portals where WebSockets fail to negotiate, pusher-js falls back to xhr_streaming.
    4. In certain corporate or proxied network environments, the HTTP streaming endpoint is intercepted by middleboxes or security layers (which return JavaScript/HTML containing class FPClassifier { ... }).
    5. pusher-js's internal XHR streaming transport blindly attempts JSON.parse(chunk.data.slice(1) || '[]') on incoming chunks, throwing an unhandled SyntaxError that bubbles up to Sentry.
    6. Moreover, NewDot uses encrypted channels (pusher-js/with-encryption), which are designed specifically for secure WebSocket connections (ws/wss). HTTP streaming fallbacks are neither intended nor suitable for Expensify's encrypted real-time architecture.

    What changes do you think we should make in order to solve the problem?

    Solution 1 (Recommended: Restrict Pusher to WebSocket transports):

    In src/libs/Pusher/index.ts:
    Configure enabledTransports: ['ws', 'wss'] in the Pusher initialization options:

     const options: Options = {
         cluster: args.cluster,
         activityTimeout: CONST.PUSHER.ACTIVITY_TIMEOUT_MS,
         pongTimeout: CONST.PUSHER.PONG_TIMEOUT_MS,
    +    enabledTransports: ['ws', 'wss'],
     };

    Why this is the cleanest solution:

    • Completely disables the legacy xhr_streaming and xhr_polling transports where the unhandled parsing crash occurs.
    • Guarantees all Pusher traffic travels exclusively over secure WebSockets (wss), which aligns with pusher-js/with-encryption requirements.
    • When connection drops occur, Pusher reconnects via WebSocket retry/backoff instead of degrading to faulty HTTP streams.

    Solution 2 (Add Sentry ignore rule as defensive measure):

    In src/libs/Log.ts or Sentry client configuration:
    Filter out SyntaxError originating from pusher-js XHR streaming transport in beforeSend:

    if (event.exception?.values?.some(ex => ex.value?.includes('FPClassifier') || ex.stacktrace?.frames?.some(f => f.filename?.includes('pusher')))) {
        return null;
    }

    Solution 1 is strongly recommended as the primary fix because disabling HTTP fallback transports eliminates the underlying crash altogether.


    What alternative solutions did you explore? (Optional)

    • Patching pusher-js via patch-package: Modifying node_modules/pusher-js to wrap JSON.parse in a try/catch. Maintaining a patch-package for a third-party dependency is unnecessary when configuring enabledTransports: ['ws', 'wss'] natively resolves the issue.
  8. abzokhattab commented on Oct 1, 2026

    @abzokhattab
    Contributor

    Melvin proposal looks good to me 🎀 👀 🎀 C+ reviewed

  9. melvin-bot commented on Oct 1, 2026

    @melvin-bot

    Current assignee @mountiny is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

  10. abzokhattab commented on Oct 1, 2026

    @abzokhattab
    Contributor

    @MelvinBot implement this with these changes:

    • Skip lines that fail to parse without emitting anything. Do not emit error / MessageParseError, because PusherConnectionManager would send it to Log.alert.
    • In the c case, also skip the line if the parsed payload is not an array.
    • Add a Jest test that feeds the HTTP stream a class FPClassifier { line before the o and a frames. Check that:
      • onopen and onmessage still fire in the same progress event.
      • onclose fires when the response ends.
    • In details.md, say the injected text comes from Forcepoint DLP, which adds its script to Pusher's application/javascript stream, and that upstream pusher-js master has the same unguarded parse.
  11. 25 remaining items

  12. abzokhattab commented on Oct 10, 2026

    @abzokhattab
    Contributor

    Still on hold for the upstream library version bump

  13. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Oct 11, 2026
  14. melvin-bot commented on Oct 11, 2026

    @melvin-bot

    Triggered auto assignment to @mallenexpensify (Awaiting Payment)

  15. melvin-bot commented on Oct 11, 2026

    @melvin-bot

    Payment Summary

    Resolving PRs:

    BugZero Checklist (@mallenexpensify)

    • I have confirmed assignees, roles, and Upwork contracts look correct
    • I have paid out Upwork contracts / manual NewDot requests
    • [BugZero Assignee] I have created a GH issue for creating/updating the regression test once above steps have been agreed upon
  16. added
    Skip PaymentSkips all the payment process steps for an issue. Use it for fixed-price projects.
    and removed
    Awaiting PaymentAuto-added when associated PR is deployed to production
    on Oct 12, 2026
  17. abzokhattab commented on Oct 12, 2026

    @abzokhattab
    Contributor

    waiting for the library version upgrade #102745 (comment)

  18. changed the title [-][Due for payment 2026-10-12] [$175] [Sentry: APP-27B] Pusher chunked JSON parse breaks on truncated payload[/-] [+][$175] [Sentry: APP-27B] Pusher chunked JSON parse breaks on truncated payload[/+] on Oct 14, 2026
  19. removed their assignment
    on Oct 14, 2026
  20. mallenexpensify commented on Oct 14, 2026

    @mallenexpensify
    Contributor

    Please comment and tag me if/when payment is due, thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorSkip PaymentSkips all the payment process steps for an issue. Use it for fixed-price projects.WeeklyKSv2

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions