[$250] Web - Attachment - Onyx and DataCloneError console rrrors when sending an image attachment

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.49-0 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/5084384&group_by=cases:section_id&group_order=asc&group_id=292107 Issue reported by: Applause Internal Team

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Log in with a new Gmail account
3. Navigate to FAB - Start chat
4. Start a 1:1 DM with any account
5. Send any image attachment to the chat

Expected Result:

There shouldn't be any console errors.

Actual Result:

Onyx and DataCloneError console errors when sending an image attachment to a 1:1 DM. Expensify.tsx:50 [Onyx] Attempted to set invalid data set in Onyx. Please ensure all data is serializable.

index.js:40 Uncaught (in promise) DataCloneError: Failed to execute 'put' on 'IDBObjectStore': function arrayBuffer() { [native code] } could not be cloned. at index.js:40:15 at index.js:13:51

Workaround:

Unknown

Platforms:

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

1510.txt

https://github.com/user-attachments/assets/389e439f-d7ec-4f4a-91d1-f6f815351388

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846724620826087266
• Upwork Job ID: 1846724620826087266
• Last Price Increase: 2024-11-14

Issue Owner

Current Issue Owner: @Pujan92

[melvin-bot[bot]]

Triggered auto assignment to @RachCHopkins (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]

@RachCHopkins 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

[RachCHopkins]

I can't repro this, I don't get any errors at all. I note the steps are very specific - does the account need to be brand new and also need to be gmail @IuliiaHerets ?

[daledah]

Proposal

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

Onyx and DataCloneError console errors when sending an image attachment to a 1:1 DM. Expensify.tsx:50 [Onyx] Attempted to set invalid data set in Onyx. Please ensure all data is serializable.

What is the root cause of that problem?

When sending attachment as first message in a new chat room, a SendAttachment request is called, but response's previousUpdateID doesn't match with request's clientUpdateID, thus trigerring SaveResponseInOnyx middleware logics:

In SaveResponseInOnyx, we save the whole request object when the above case happens:

https://github.com/Expensify/App/blob/927b40d5f93ea7cdd20bf4a7eb9ab8970b994781/src/libs/Middleware/SaveResponseInOnyx.ts#L26-L32

https://github.com/Expensify/App/blob/927b40d5f93ea7cdd20bf4a7eb9ab8970b994781/src/libs/Middleware/SaveResponseInOnyx.ts#L39

In the case of SendAttachment request, the request contains a File object, which is a non-serializable object, which makes Onyx unable to set request values into OnyxDB.

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

1. We can handle this in BE side to response AddAttachment with previousUpdateID matches with clientUpdateID to prevent SaveResponseInOnyx being called
2. We can serialize File object by wrapping request around JSON.stringify and JSON.parse: https://github.com/Expensify/App/blob/927b40d5f93ea7cdd20bf4a7eb9ab8970b994781/src/libs/Middleware/SaveResponseInOnyx.ts#L30

request: JSON.parse(JSON.stringify(request))

We can limit this logic to apply only on SendAttachment as well

const shouldParseRequest = request.command === WRITE_COMMANDS.ADD_ATTACHMENT
/* ... */
request: shouldParseRequest ? JSON.parse(JSON.stringify(request)) : request

What alternative solutions did you explore? (Optional)

[daledah]

@RachCHopkins From my testings, you can try to reproduce with the following conditions:

• Start a chat with an existing user, not optimistic user
• Send image as the first message in the chat

[RachCHopkins]

Strange, no, I can't repro. But I trust you @daledah, if you can, then that's good enough for me!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@Pujan92, @RachCHopkins Eep! 4 days overdue now. Issues have feelings too...

[RachCHopkins]

@Pujan92 can you check out the above proposal, please?

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[huult]

Proposal

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

Onyx and DataCloneError console errors when sending an image attachment

What is the root cause of that problem?

This issue related RCA with issue 49329 and 44114

This issue occurs on the backend because when we call the AddAttachment request, the lastUpdateIDFromClient is less than the previousUpdateID received. As a result, the responseToApply ends up using the wrong function to store the responseToApply, which is OnyxUpdates.saveUpdateInformation(responseToApply). In this case, we expect to use OnyxUpdates.apply(responseToApply); to save the response. Additionally, when we use this function, the responseToApply from AddAttachment needs to be serialized first; otherwise, it will throw an error log in this ticket.

{"lastUpdateIDFromClient":2571094520,"previousUpdateID":2571094574}

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

To resolve this issue, we need to make a change on the backend so that the previousUpdateID is less than the lastUpdateIDFromClient in this case, or we should always update ADD_ATTACHMENT to the most current state. Something like this:

//.src/libs/Middleware/SaveResponseInOnyx.ts#L12
const requestsToIgnoreLastUpdateID: string[] = [
    WRITE_COMMANDS.OPEN_APP,
    SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP,
    WRITE_COMMANDS.CLOSE_ACCOUNT,
+    WRITE_COMMANDS.ADD_ATTACHMENT,
    WRITE_COMMANDS.DELETE_MONEY_REQUEST,
    SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES,
];

[melvin-bot[bot]]

@Pujan92, @RachCHopkins 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[RachCHopkins]

@Pujan92 how do you feel about these proposals?

[Pujan92]

Reviewing

[Pujan92]

As both proposals stated, I think this needs to be fixed in the BE if possible, to correct the values of previousUpdateID and lastUpdateIDFromClient. So that the correct function OnyxUpdates.apply gets called. Adding an internal engineer for the quick look.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[rlinoz]

Im not very familiar with the updateIDs logic, but I will try to take a look today.

[huult]

If the backend doesn't have a solution to update it, I think we can still fix this on the frontend, and we can add it to requestsToIgnoreLastUpdateID because, upon checking, it seems we don't need to wait for any requests to update fields in the add attachment request.

[rlinoz]

Im not being able to repro this 🤔

[huult]

@rlinoz , I would like to share the video with you so you know how I reproduced this issue

Video

https://github.com/user-attachments/assets/e62819a0-408d-487b-a16f-a59d203d293e

1. Log in with a new account.
2. Start a direct message.
3. Send an image.
4. Observe the error message after sending the image.

[Pujan92]

1. Log in with a new account.

No need to log in with a new account, sending an image as the first message to a new contact also resulted in a console error.

https://github.com/user-attachments/assets/d189e077-f1d3-4c16-ac23-fa7f2bb4399c

[melvin-bot[bot]]

@rlinoz @Pujan92 @RachCHopkins this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[rlinoz]

If I follow https://github.com/Expensify/App/issues/50780#issuecomment-2443249515

Let me try this: https://github.com/Expensify/App/issues/50780#issuecomment-2442990320

[rlinoz]

same thing, are you trying in staging?

[huult]

@rlinoz I'm still able to reproduce this issue on staging.

https://github.com/user-attachments/assets/b5d11d4f-0782-499a-82cd-9a77645c340a

[daledah]

@rlinoz You can try to reproduce using conditions mentioned in https://github.com/Expensify/App/issues/50780#issuecomment-2415907266:

• Start a chat with an existing user, not optimistic user
• Send image as the first message in the chat

https://github.com/user-attachments/assets/f1d8e789-3aa8-410c-99c3-d9a0f1b1d1ad

[rlinoz]

I am having a really hard time trying to reproduce this, let me know if I am missing something in the video

https://github.com/user-attachments/assets/78944310-5abc-4660-8f62-e2b5039edd9b

Looking at the logs that I think are from the comment above I see the previousUpdateID higher than the clientUpdateID

clientUpdateID: 2660128276 previousUpdateID: 2660128284

And looking at the logs from my own request the same thing happens but no console error

clientUpdateID: 2660954499 previousUpdateID: 2660954506

But looking at random AddComment requests this seems to often be true, so I am not sure the updateID is wrong here.

[daledah]

@rlinoz I'm leaning towards the possibility of browser-specific bug here. Tested on Safari and the error doesn't appear, instead I had another error:

You can try to reproduce using Chrome.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[rlinoz]

Nothing on latest chrome version:

Anyway, maybe we can make sure that this comment is corret: https://github.com/Expensify/App/issues/50780#issuecomment-2441712805

[huult]

Anyway, maybe we can make sure that this comment is corret: https://github.com/Expensify/App/issues/50780#issuecomment-2441712805

@rlinoz I tested DM, IOU and sent an attachment there. I see that ADD_ATTACHMENT doesn’t wait for any request to update the field in the add attachment request

VIDEO

https://github.com/user-attachments/assets/5723fb1e-989d-43a1-a621-1c4d347b9275

[melvin-bot[bot]]

@rlinoz, @Pujan92, @RachCHopkins Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rlinoz]

@Pujan92 thoughts on the above? https://github.com/Expensify/App/issues/50780#issuecomment-2452089309

[Pujan92]

@rlinoz I think we can proceed with what @huult suggested which is to add command in requestsToIgnoreLastUpdateID.

[daledah]

@Pujan92 @rlinoz This bug happens on RequestMoney as well. Steps:

1. Create a WS
2. In the WS chat, submit a receipt expense

https://github.com/user-attachments/assets/f3836ecd-3d76-4775-b4ed-c851e263cb6e

There might be many requests like this, and I think it's not a good idea to put all of them into requestsToIgnoreLastUpdateID.

[rlinoz]

So we basically shouldn't be trying to set a File into the OnyxDB right?

[daledah]

@rlinoz We should either not to add Files into OnyxDB or we serialize it first.

[wildan-m]

Proposal

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

Console errors, Onyx and DataCloneError, occur when sending an image attachment on the web.

What is the root cause of that problem?

We are trying to store blob file here:

https://github.com/Expensify/App/blob/afa7b33107747d0242acfd01d032d5339be390d9/src/libs/Middleware/SaveResponseInOnyx.ts#L38-L39

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

[!NOTE]
If this solution has not been officially proposed, I propose it; if it is considered a duplicate, I vote for it.

If we can't store a file in Onyx, we shouldn't do so. Preprocessing the file is unnecessary if we don't actually use it later.

We can create filterFiles function to recursively filter files in request.data

function filterFiles(obj: any): any {
    if (Array.isArray(obj)) {
        return obj.map(filterFiles);
    } else if (obj && typeof obj === 'object') {
        return Object.keys(obj).reduce((acc, key) => {
            if (obj[key] instanceof File) {
                return acc;
            }
            acc[key] = filterFiles(obj[key]);
            return acc;
        }, {} as any);
    }
    return obj;
}

And modify responseToApply

        const filteredRequest = {
            ...request,
            data: filterFiles(request.data),
        };

        const responseToApply = {
            type: CONST.ONYX_UPDATE_TYPES.HTTPS,
            lastUpdateID: Number(response?.lastUpdateID ?? 0),
            previousUpdateID: Number(response?.previousUpdateID ?? 0),
            request: filteredRequest,
            response: response ?? {},
        };

Branch for this solution

What alternative solutions did you explore? (Optional)

By following the YAGNI principle, we can whitelist stored request.data to avoid storing unnecessary data.

Add to CONST

    ONYX_ALLOWED_REQUEST_DATA: ['apiRequestType', 'returnValueList', 'nvpNames', 'shouldRetry', 'apiRequestType', 'skipReauthentication', 'shouldProcessImmediately', 'canCancel'], // this just example

Modify responseToApply

const filterAllowedRequestData = (data: any, allowedKeys: string[]) => {
    const regex = new RegExp(`^(${allowedKeys.join('|')})$`);
    return Object.keys(data)
        .filter(key => regex.test(key))
        .reduce((obj, key) => {
            obj[key] = data[key];
            return obj;
        }, {});
};

...

        const filteredRequestData = filterAllowedRequestData(request.data, [...CONST.ONYX_ALLOWED_REQUEST_DATA]);

        const responseToApply = {
            type: CONST.ONYX_UPDATE_TYPES.HTTPS,
            lastUpdateID: Number(response?.lastUpdateID ?? 0),
            previousUpdateID: Number(response?.previousUpdateID ?? 0),
            request: {
                ...request,
                data: filteredRequestData,
            },
            response: response ?? {},
        };

Branch for this solution

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[Pujan92]

If we can't store a file in Onyx, we shouldn't do so

@wildan-m I see the file object stored for the onyx key networkRequestQueue if we try to send an attachment offline.

[wildan-m]

@Pujan92 I can no longer reproduce it. Can you?

[Pujan92]

Yes, I am still able to reproduce it.

[melvin-bot[bot]]

@rlinoz @Pujan92 @RachCHopkins this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

@rlinoz, @Pujan92, @RachCHopkins Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rlinoz]

so why are we not being able to save a file in this case? 🤔

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸