[HOLD for payment 2022-01-21] [$1000] When sending new chats in Desktop, an old message persists at the bottom

[mallenexpensify]

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

---

Action Performed:

1. Open the desktop app
2. Switch wifi network from home to tethering
3. Send some messages

Expected Result:

New messages should always display at the bottom.

Actual Result:

When sending new chats in Desktop, an old message persists (is stuck) at the bottom.

Workaround:

Hard refreshing used to solve the problem but it's not now (at least not consistently on desktop). A user needs to log out/in to fix

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App ✔️
Mobile Web

Version Number: Version 1.1.7-24

Notes/Photos/Videos:
Switch wifi network from my home network to tethering from my phone, then send messages.

2021-10-19_14-23-38.mp4

Original issue is #3623, it got messy so I created this fresh one.

Logs:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @mallenexpensify https://expensify.slack.com/archives/C01GTK53T8Q/p1623805090370500

When sending new chats in Desktop, an old message persists at the bottom.

[MelvinBot]

Triggered auto assignment to @conorpendergrast (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[mallenexpensify]

@isagoico Can you do me a favor and see if you can reproduce based on my steps above?

[MelvinBot]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[MelvinBot]

Huh... This is 4 days overdue. Who can take care of this?

[mallenexpensify]

@isagoico Can you do me a favor and see if you can reproduce based on my steps above?

[isagoico]

@mallenexpensify Still unable to reproduce this on my side on the Desktop app

[MelvinBot]

Eep! 4 days overdue now. Issues have feelings too...

[johnmlee101]

Makes sense, I think handling "no response" cases seems like a good next progression of our network logic.
Assigning @kidroca !

[MelvinBot]

📣 @kidroca You have been assigned to this job by @johnmlee101!
Please apply to this job in Upwork and let us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[kidroca]

Should I be posting a comment here once I apply to Upwork?

It's not specified on https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#propose-a-solution-for-the-job

If your solution proposal is accepted, Expensify will hire you on Upwork and assign the GitHub issue to you.

I've only been assigned the issue, but not been hired in upwork

---

Please apply to this job in Upwork and let us know when we can expect a PR to be ready for review 🧑‍💻

Does that mean I should post a comment here or on Upwork? Because on the proposal form in Upwork there's:

What date do you estimate to be able to submit a pull request containing the fix for this issue?

[kadiealexander]

Jumping in while Matt's on leave.

@kidroca I see how this is confusing! I'll pass this feedback along to the team. Could you please leave a comment here with the expected completion time? I've gone ahead and hired you in Upwork.

[kidroca]

Thanks @kadiealexander
I expect to submit a PR by 2021-12-02 EOD

[kidroca]

PR is ready: #6567

[mallenexpensify]

Pinged John and Rory in the PR for 👀 #6567 (comment)

[johnmlee101]

Going to review this soon! However a lot of this is outside of my wheelhouse a bit more than normal, so I definitely want to have Rory review as well.

[mallenexpensify]

@johnmlee101 @roryabraham can one of you review the PR when ya have time, @kidroca 's been waiting over two weeks (and yes... it's the holidays....)

[mallenexpensify]

it's moving along!!!

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.29-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Network: Retry requests that failed in flight #6567

If no regressions arise, payment will be issued on 2022-01-21. 🎊

[mallenexpensify]

Posted the below image in this slack thread

@kidroca do you think the issue I had was related to this GH? In the Slack thread I said 'do nothing' for now so we can wait to see if it happens again

[kidroca]

@kidroca do you think the issue I had was related to this GH? In the Slack thread I said 'do nothing' for now so we can wait to see if it happens again

It kinda is because looks like it would create the same problem - a message stuck on the bottom

But the focus of the current issue was on "Network not present" - our messages never reaching the server - and not the server returning 50x codes so we never explored this possibility

Whatever message you sent - either caused the server to choke and be unable to process it, or you message reached the server but it wasn't able to return a response because something else already preoccupied it
504 - means that you reached a server, it proxied your request upstream, but it didn't get a response in a reasonable time (15 sec?)

Maybe we need another ticket to find out why a 504 was returned, should we worry about that?

[kidroca]

I've followed the code and it seems we're covering this case as well - meaning App should re-try posting you're Report Action and eventually succeed

1 When we get a 50x error this method would typically fail due to response.json() and no JSON in the response

• so far I haven't seen the backend return JSON content when the HTTP response code is 50x
• In the console log above you can see a 504 resulted in an Uncaught (in promise) Error: session.offlineMessageRetry

App/src/libs/HttpUtils.js

Lines 14 to 28 in dc2b32e

	* Send an HTTP request, and attempt to resolve the json response.
	* If there is a network error, we'll set the application offline.
	*
	* @param {String} url
	* @param {String} method
	* @param {Object} body
	* @returns {Promise}
	*/
	function processHTTPRequest(url, method = 'get', body = null) {
	return fetch(url, {
	method,
	body,
	})
	.then(response => response.json());
	}

2 Which means we're going to go in the catch handler here:

App/src/libs/Network.js

Lines 228 to 239 in dc2b32e

	HttpUtils.xhr(queuedRequest.command, finalParameters, queuedRequest.type, queuedRequest.shouldUseSecure)
	.then(response => onResponse(queuedRequest, response))
	.catch((error) => {
	// When the request did not reach its destination add it back the queue to be retried
	const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry');
	if (shouldRetry) {
	networkRequestQueue.push(queuedRequest);
	return;
	}
	onError(queuedRequest, error);
	});

3 The Report_AddComment action is retriable and it probably got retired.

Non retrievable actions would fall to onError(queuedRequest, error) and this will print Uncaught (in promise) Error: session.offlineMessageRetry

---

So the comment would stay on the bottom, but it will get delivered eventually

[mallenexpensify]

Thanks for the context and explanation, it was helpful for me, being a non-engineer.

So the comment would stay on the bottom, but it will get delivered eventually

This likely explains why the comment stopped being stuck at the bottom soon after. In the past, the chats used to get stuck for more longer, I think I had to sign out/in to get them unstuck.

Maybe we need another ticket to find out why a 504 was returned, should we worry about that?

Can you think of a way to reproduce the issue I had? If so, we should definitely create an issue. If not.. are there other way to 'dig into' why 504 was possibly returned? Then we'd be able to potentially make a fix.

[kidroca]

Can you think of a way to reproduce the issue I had?

No

If not.. are there other way to 'dig into' why 504 was possibly returned?

I'm 99% sure there should be server logs about these 50x errors and someone already monitors them and would do something about it if necessary or bring it up internally

Maybe the request timed out because you send a message just as the backend was getting updated?

In any case it something requiring internal knowledge and it's either not a problem or should be discussed internally

[mallenexpensify]

Thanks @kidroca I've asked QA to create a new issue, reference this one and to label Internal and Needs Reproduction

https://expensify.slack.com/archives/C01GTK53T8Q/p1642529744136800?thread_ts=1642034861.416800&cid=C01GTK53T8Q

[mallenexpensify]

Paid @kidroca $1000