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 2024-01-17] [HOLD for payment 2024-01-18] Chat - The app crashes if I open an image attachment in offline mode #34047

Closed
2 of 6 tasks
lanitochka17 opened this issue Jan 5, 2024 · 43 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@lanitochka17
Copy link

lanitochka17 commented Jan 5, 2024

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: 1.4.22.0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Open a conversation
  2. Send an image attachment
  3. Go offline
  4. Tap on the attachment

Expected Result:

The app shouldn't crash

Actual Result:

The app crashes if I open an image attachment in offline mode

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6333328_1704473177929.az_recorder_20240105_101313.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Jan 5, 2024
Copy link
Contributor

github-actions bot commented Jan 5, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Jan 5, 2024

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

@0xmiros
Copy link
Contributor

0xmiros commented Jan 5, 2024

I am not able to reproduce on main. Tested on android physical device

@0xmiros
Copy link
Contributor

0xmiros commented Jan 5, 2024

@lanitochka17 can you please share crash log?

@lanitochka17
Copy link
Author

@0xmiroslav Made a request to the tester

@lanitochka17
Copy link
Author

@0xmiroslav
log.txt

@NikkiWines
Copy link
Contributor

I can't reproduce this on staging (iOS 1.4.22.0 and 1.4.22.1) or on dev 🤔

RPReplay_Final1704495691.mov

@situchan
Copy link
Contributor

situchan commented Jan 5, 2024

Checking crash log, this happens on pdf attachment and dupe of #34017

crash

Here's context: #34017 (comment)

@NikkiWines I am available now to raise PR

@NikkiWines
Copy link
Contributor

oh nice, good catch on the dupe @situchan. It looks like we updated to 6.7.4 because of another issue related to the app crashing with PDFs (#33561) so I'm not sure downgrading is in our best interest 🤔

I guess it depends on which scenario we think would be more common:
a. User viewing a PDF file in offline mode
b. User uploading a PDfs with formulas/maths symbols

@situchan
Copy link
Contributor

situchan commented Jan 5, 2024

#29743 is android only issue and happened on specific pdfs.
#34047 is native (android/iOS) issue and happens on all pdfs.
So I believe #34047 is more common scenario

@marcaaron
Copy link
Contributor

hmm I am kind of confused actually and questioning the relatedness now. Tester is not using PDFs are they?

@situchan
Copy link
Contributor

situchan commented Jan 5, 2024

The clear repro step is #34017

@marcaaron
Copy link
Contributor

This issue does not reference anything about a PDF.

@situchan
Copy link
Contributor

situchan commented Jan 6, 2024

  1. Go to the chat where you have sent the PDF file
  2. Send any image
  3. Tap on the image preview
  4. Tap on the back button

I believe @lanitochka17 missed Step 1 in repro step.
I checked crash log in #34047 (comment) and crash happened on pdf.

@marcaaron
Copy link
Contributor

Not seeing it. How did you determine that?

@situchan
Copy link
Contributor

situchan commented Jan 6, 2024

Not seeing it. How did you determine that?

crash

@situchan
Copy link
Contributor

situchan commented Jan 6, 2024

As no one is able to reproduce, I think we can ask QA team test staging build again after fixing pdf crash.

@marcaaron
Copy link
Contributor

Nah, I just reproduced on Android with a PDF. They are the same 👍👍

@marcaaron
Copy link
Contributor

I guess it depends on which scenario we think would be more common:
a. User viewing a PDF file in offline mode
b. User uploading a PDfs with formulas/maths symbols

Maybe one of these:

  1. Remove the blocker and wait for the upstream fix.
  2. Don't try to load any PDFs if we are offline. Better than a crash.

@situchan
Copy link
Contributor

situchan commented Jan 6, 2024

Don't try to load any PDFs if we are offline. Better than a crash.

This happens even when online.
#34017 which was closed as dupe has repro step in online mode

@situchan
Copy link
Contributor

situchan commented Jan 6, 2024

online.test.mov

@NikkiWines NikkiWines added the Daily KSv2 label Jan 9, 2024
@NikkiWines
Copy link
Contributor

Updating the labels for this one so we don't forget to issue payment

@NikkiWines NikkiWines changed the title Chat - The app crashes if I open an image attachment in offline mode [HOLD for payment 2024-01-18] Chat - The app crashes if I open an image attachment in offline mode Jan 10, 2024
@NikkiWines NikkiWines added Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. labels Jan 10, 2024
Copy link

melvin-bot bot commented Jan 10, 2024

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@NikkiWines NikkiWines assigned situchan and unassigned lschurr Jan 10, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 10, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-01-18] Chat - The app crashes if I open an image attachment in offline mode [HOLD for payment 2024-01-17] [HOLD for payment 2024-01-18] Chat - The app crashes if I open an image attachment in offline mode Jan 10, 2024
Copy link

melvin-bot bot commented Jan 10, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.23-4 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 2024-01-17. 🎊

For reference, here are some details about the assignees on this issue:

  • @situchan requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Jan 10, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@situchan] The PR that introduced the bug has been identified. Link to the PR:
  • [@situchan] 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:
  • [@situchan] 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:
  • [@situchan] Determine if we should create a regression test for this bug.
  • [@situchan] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Copy link

melvin-bot bot commented Jan 17, 2024

Issue is ready for payment but no BZ is assigned. @puneetlath you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

Copy link

melvin-bot bot commented Jan 17, 2024

Payment Summary

Upwork Job

  • ROLE: @situchan paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@puneetlath)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@puneetlath
Copy link
Contributor

Hm, what's due here? Is this the PR that @situchan should be paid for? #34078

@situchan
Copy link
Contributor

yes

@puneetlath
Copy link
Contributor

And how much is due? $250 since this was a straight revert? Or something else?

@situchan
Copy link
Contributor

I am not sure if this case applies here.

Though solution was straight revert, there was an effort to reproduce and find the root cause.

@melvin-bot melvin-bot bot added the Overdue label Jan 19, 2024
@puneetlath
Copy link
Contributor

Ok sounds good, thanks for the link.

Payment summary:

Please ping me on the issue when you've accepted.

@melvin-bot melvin-bot bot removed the Overdue label Jan 23, 2024
@situchan
Copy link
Contributor

Accepted thanks

@puneetlath
Copy link
Contributor

Paid. Thanks everyone!

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 Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering
Projects
None yet
Development

No branches or pull requests

10 participants