Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2022-03-22] [$500] Connection drop is not detected - Reported by: @Santhosh-Sellavel #6059

Closed
isagoico opened this issue Oct 26, 2021 · 77 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review

Comments

@isagoico
Copy link

isagoico commented Oct 26, 2021

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. Steps to simulate connect loss:
  2. Turn on mobile internet on phone.
  3. Turn on hotspot/internet sharing on your phone.
  4. Connect your laptop/pc to the network shared from mobile.
  5. Use the new Expensify app.
  6. Now disable mobile internet in the phone, now its connection drop but our app doesn’t detect it.
  7. You can verify opening any user chat, we can't see the offline message warning.

Expected Result:

Connection drop should be detected on all circumstances.

Actual Result:

Connection drop is not detected and the app should online status.

Workaround:

None, going back online fixes the issue.

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.1.9-1

Reproducible in staging?: Yes
Reproducible in production?: Yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1634766280030400

Job: https://www.upwork.com/jobs/~0129cfc8254ac008fa

View all open jobs on GitHub

@MelvinBot
Copy link

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

@Gonals Gonals added Weekly KSv2 Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor and removed Daily KSv2 labels Oct 26, 2021
@MelvinBot
Copy link

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

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Oct 26, 2021
@Gonals
Copy link
Contributor

Gonals commented Oct 26, 2021

Nice catch! Added tags and sending to the pool

@Gonals Gonals removed their assignment Oct 26, 2021
@kadiealexander
Copy link
Contributor

Just going to reassign since I'll be OOO for the next wee while.

@kadiealexander kadiealexander removed their assignment Oct 27, 2021
@kadiealexander kadiealexander added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Oct 27, 2021
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@kevinksullivan
Copy link
Contributor

I was unable to reproduce this on web. The app responded correctly when I turned off hotspot.

image

@MelvinBot MelvinBot added Overdue and removed Overdue labels Nov 2, 2021
@Santhosh-Sellavel
Copy link
Collaborator

Sorry couldn’t able to get here early @kevinksullivan

  1. Create hotspot using mobile internet
  2. Connect the Laptop to the hotspot
  3. Now Turn off the mobile internet not the created hotspot!

@MelvinBot
Copy link

@kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@kevinksullivan
Copy link
Contributor

When you say mobile internet do you mean wifi , cellular, or something else?

@MelvinBot MelvinBot removed the Overdue label Nov 8, 2021
@Santhosh-Sellavel
Copy link
Collaborator

Cellular data @kevinksullivan

@kidroca
Copy link
Contributor

kidroca commented Feb 15, 2022

Applied on Upwork,
You can expect a PR submitted by 2022-02-18 EOD

@kidroca
Copy link
Contributor

kidroca commented Feb 19, 2022

PR is ready: #7825

@kevinksullivan
Copy link
Contributor

PR still in review.

@MelvinBot MelvinBot removed the Overdue label Mar 2, 2022
@pecanoro pecanoro added the Reviewing Has a PR in review label Mar 4, 2022
@PauloGasparSv
Copy link
Contributor

Very similar issue #7975 was fixed by the P.R. found in this issue.

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 15, 2022
@botify
Copy link

botify commented Mar 15, 2022

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

If no regressions arise, payment will be issued on 2022-03-22. 🎊

@botify botify changed the title [$500] Connection drop is not detected - Reported by: @Santhosh-Sellavel [HOLD for payment 2022-03-22] [$500] Connection drop is not detected - Reported by: @Santhosh-Sellavel Mar 15, 2022
@botify botify removed the Weekly KSv2 label Mar 22, 2022
@MelvinBot MelvinBot added the Daily KSv2 label Mar 22, 2022
@kevinksullivan
Copy link
Contributor

@kidroca @Santhosh-Sellavel looks like the job expired so I created a new one and invited you. Once you submit I will hire and pay out the $500. Thanks!

https://www.upwork.com/jobs/~0115bb5b01faa7d4b8

@parasharrajat
Copy link
Member

@kevinksullivan Should I apply on the same for C+?

@MelvinBot
Copy link

@pecanoro, @kidroca, @kevinksullivan, @parasharrajat Whoops! This issue is 2 days overdue. Let's get this updated quick!

@kidroca
Copy link
Contributor

kidroca commented Mar 30, 2022

This is actually completed, I've applied to the new ticket and received payment

@pecanoro
Copy link
Contributor

Closing then!

@parasharrajat
Copy link
Member

But Reporter and C+ are still not paid @kevinksullivan.

@Santhosh-Sellavel
Copy link
Collaborator

@kevinksullivan

I too received payment but for this, only C+ is missed out here for @parasharrajat.

@parasharrajat
Copy link
Member

@kevinksullivan Bump.

@kevinksullivan
Copy link
Contributor

Apologies @parasharrajat . Just hired, please accept and I'll get this payment over.

@parasharrajat
Copy link
Member

@kevinksullivan Done.

@kbecciv
Copy link

kbecciv commented Apr 19, 2022

@kidroca @kevinksullivan We reported the issue during Regression #8690

@kidroca
Copy link
Contributor

kidroca commented Apr 19, 2022

I've taken a look and it seems the Network code was (or is being) refactored
At the time this issue closed the functionality worked correctly

If you want me to investigate further, please tag me on the new ticket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests