[$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 [email protected] 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
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

1510.txt

Bug6634941_1728975190721.bandicam_2024-10-15_08-47-55-299.mp4

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-10-31

Issue Owner

Current Issue Owner: @Pujan92

[melvin-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:

App/src/libs/Middleware/SaveResponseInOnyx.ts

Lines 26 to 32 in 927b40d

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

App/src/libs/Middleware/SaveResponseInOnyx.ts

Line 39 in 927b40d

OnyxUpdates.saveUpdateInformation(responseToApply);

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:
   

   App/src/libs/Middleware/SaveResponseInOnyx.ts

   Line 30 in 927b40d

   request,

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]

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

[melvin-bot]

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

[melvin-bot]

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

[RachCHopkins]

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

[melvin-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]

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

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

Screen.Recording.2024-10-29.at.08.38.37.mp4

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.

Screen.Recording.2024-10-29.at.10.57.23.mov

[melvin-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 #50780 (comment)

Let me try this: #50780 (comment)

[rlinoz]

same thing, are you trying in staging?

[huult]

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

Screen.Recording.2024-10-30.at.13.16.33.mp4

[daledah]

@rlinoz You can try to reproduce using conditions mentioned in #50780 (comment):

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

Screen.Recording.2024-10-30.at.18.11.13.mp4

[rlinoz]

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

Screen.Recording.2024-10-30.at.09.30.50.mov

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]

📣 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: #50780 (comment)

[huult]

Anyway, maybe we can make sure that this comment is corret: #50780 (comment)

@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

Screen.Recording.2024-11-01.at.22.40.28.mp4

[melvin-bot]

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

[rlinoz]

@Pujan92 thoughts on the above? #50780 (comment)

[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

Screen.Recording.2024-11-04.at.08.37.10.mov

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?