[HOLD for payment 2024-09-16] [$250] Account Settings- App freezes after sending a log file

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v9.0.20-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4859439
Issue reported by: Applause Internal Team

Action Performed:

1. Go to Account Settings -> Troubleshoot
2. Enable "Client side logging"
3. Press "View debug console"
4. Press "Share log"
5. Press on any conversation

Expected Result:

The user should be moved to the report and logs should be sent as an attachment.
The app is stable and functional to return to the previous screen or access the user's profile.

Actual Result:

The user is moved to the report and logs are sent as an attachment.
The back button and the user's profile function become unresponsive. The app sometimes crashes after a few minutes.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Log_1508.txt

Bug6573040_1723736646284.Sending_log_file.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016f26106fe45b6825
• Upwork Job ID: 1825662708863963045
• Last Price Increase: 2024-08-19
• Automatic offers:
• suneox | Reviewer | 103607773
• dominictb | Contributor | 103607774

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

We think that this bug might be related to #wave-collect RELEASE 1

[IuliiaHerets]

@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[dominictb]

Edited by proposal-police: This proposal was edited at 2023-10-02T10:00:00Z.

Proposal

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

Step to reproduce the issue (aside from OP's step)

• Open app
• Try to send an .txt attachment (One way to obtain it is exporting the onyx client log in Troubleshooting menu)
• Observe that the app freeze / crash (sometimes), and in the console infinite errors happen

What is the root cause of that problem?

We are trying to add log to Onyx in here, but the extraData can be non-serializable. One possible way that the extraData is non-serializable is due to this, as in this case, the request contains a File-type field.

The problem is that at first, the file field in AddAttachment API request params is still serializable (doesn't contain arraybuffer/stream data type), hence the request data is somehow persisted in Onyx (both in cache storage and in keyvalue store). However, after the request data is added to the FormData and sent, the file object now contains arrayBuffer and other binary/stream types which is non-serializable, hence making the addLog Onyx operation to fail.

We can explain the non-stop error message log by following: Since the request is thrown in a promise chain without a proper catch, it will bubble up to the

App/src/components/ErrorBoundary/index.tsx

Line 18 in d7c9087

if (typeof event.reason === 'object' && event.reason !== null) {

, which again trying to produce another log and added it to Onyx. The problem is, the current Onyx pre-merged value for the OnyxKeys.Log field contains File-type data in the cache, and Onyx is trying to set the merged data (including the File data in the indexedDB storage -> causing it the serializable issue as described in the screenshot above)., and the error will bubble back to the ErrorBoundary -> infinite circle.

Extra note: Theoretically, we also encounter this issue while persisting AddAttachment request in Onyx storage, but it's hardly reproducible, and we see in practice that at first, the File object is still serializable, but then once the request is successfully executed, it will be removed forever, hence, the File object doesn't entail in the Onyx cache, causing further issue for subsequent merge/mergeCollection operations. However, with logging in Onyx, it is File object is still in the cache for a while, hence subsequent merge/mergeCollections will bump into this error.

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

We should try to serialize the request.data before adding to the log. A holistic function can be enough. We can add more support for some special data type like File or Blob, or tune the implementation details based on our needs.

// in @libs/Middleware/Logging.ts

function serializeValue(value: any): string {
    const seen = new WeakSet();
  
    function serialize(item: any): any {
      // Handle primitive types
      if (item === undefined || item === null) return item
      if (typeof item === "number" || typeof item === "boolean") return item
      if (typeof item === "string") return item
  
      // Handle special objects
      if (item instanceof Date) return item;
      if (item instanceof RegExp) return item;
  
      // Handle arrays
      if (Array.isArray(item)) {
        if (seen.has(item)) return "\"[Circular]\"";
        seen.add(item);
        const serializedArray = item.map(serialize);
        seen.delete(item);
        return serializedArray;
      }
  
      // Handle objects
      if (typeof item === "object") {
        if (seen.has(item)) return "\"[Circular]\"";
        seen.add(item);
        const entries = Object.fromEntries(Object.entries(item).map(([key, val]) => [key, serialize(val)]));
        seen.delete(item);
        return entries;
      }
  
      // Handle functions and other non-serializable types
      const type = item.constructor.name || Object.prototype.toString.call(item).slice(8, -1);
      return `"[Not serializable: ${type}]"`;
    }
  
    return serialize(value);
  }

function logRequestDetails(message: string, request: Request, response?: Response | void) {
    ....
    const extraData: Record<string, unknown> = {};
    /**
     * We don't want to log the request and response data for AuthenticatePusher
     * requests because they contain sensitive information.
     */
    if (request.command !== 'AuthenticatePusher') {
        extraData.request = {
            ...request,
            data: request.data,
        }
        extraData.response = response;
    }

    Log.info(message, false, logParams, false, extraData);
}

What alternative solutions did you explore? (Optional)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016f26106fe45b6825

[melvin-bot]

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

[strepanier03]

Able to repro in both mweb/Chrome and MacOS.

[strepanier03]

Finally crashed on me:

[suneox]

@dominictb proposal LGTM. After enabling client-side logging, every request/response will be stored. However, since IndexedDB on the browser use IDBObjectStore and stores the cloned value via the structuredClone function, it does not support cloning function.

Therefore, we can proceed with this proposal.

Note: The serializeValue function from the selected proposal is missing support for some supported types, but we can address that when creating the PR.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @puneetlath, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[trjExpensify]

Not sure why this was put in #wave-collect, moving it to #newdot-quality as it's related to a troubleshooting feature.

[melvin-bot]

This issue has not been updated in over 15 days. @puneetlath, @strepanier03, @suneox, @dominictb eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[dominictb]

@strepanier03 PR hit production on Sep 9 #48664 (comment). Should be ready for payment now.

[suneox]

Bugzero Checklist

• [@suneox] The PR that introduced the bug has been identified. Link to the PR: feat: log network response and request with filter options in debug console #45769
• [@suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment
• [@suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [@suneox] Determine if we should create a regression test for this bug: N/A: This issue has handled an edge case when updating the log request.​

[suneox]

@strepanier03 The checklist is completed, and we can proceed with the payment.​

[dominictb]

@puneetlath Since this is awaiting payment. Can you remove the Monthly and Reviewing labels and add Daily?

[trjExpensify]

Done that for you, @dominictb.

[melvin-bot]

@puneetlath, @strepanier03, @suneox, @dominictb Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[puneetlath]

@strepanier03 looks like this is awaiting payment from you.

[strepanier03]

Both contracts paid and closed with feedback left. I was OoO for a bit and this came due during that time, then the Hold status had it hidden from me and I missed it. Thank you for the bumps.