[$250] uncaught exception error when uploading a video

[m-natarajan]

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: 1.4.55-0
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
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: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710897693150179

Action Performed:

1. Go to any chat
2. upload a video
3. Press play

Expected Result:

A console error doesn't occur when you hit play.

Actual Result:

Console error "uncaught exception "appears after hitting play.

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016b94a07bd4d3ae52
• Upwork Job ID: 1776394806990274560
• Last Price Increase: 2024-04-05
• Automatic offers:
• s77rt | Reviewer | 0

[melvin-bot]

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

[abekkala]

I was not able to reproduce

My video loaded with no console error

[abekkala]

let's continue to test to see if we can pinpoint if the the previous error appears again and then figure out why

[melvin-bot]

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

[abekkala]

@mallenexpensify I'm ooo until April 08 - I'll be taking back any issues that are still Open when I return.

---

CURRENT STATUS:

I was unable to reproduce this. but I've applied the re-test weekly label to further test if they can reproduce what David experienced. As he seemed pretty insistent on pinpointing the cause of the JS console error he received. https://expensify.slack.com/archives/C049HHMV9SM/p1710897693150179

[mallenexpensify]

also unable to reproduce, waiting for QA to retest

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[mallenexpensify]

Added Needs Reproduction label. I plan to address these types of issues soon so I'm leaving open

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[mvtglobally]

Issue is reproducible during KI retests.

1711909850032.bandicam_2024-03-31_23-59-12-295.mp4

[serhii1030]

@s77rt

@KMichel1030 Can you please update your proposal to cover the suggested solution?

Could you please let me know which method do you mean? Sorry for the misunderstanding.
1.
Reset currentVideoPlayerRef when opening menu
2.

But I think downloading or changing play speed of one video should not affect other video(that is playing).
In this case we can implement playback speed in similar method with downloading.
Please let me know your suggestion.

[s77rt]

@KMichel1030 The first one

[serhii1030]

@s77rt
I've updated Proposal with your suggestion.

[s77rt]

@KMichel1030 Thanks for the update. Overall the solution looks good but I have a question about

Add function in PlaybackContext.tsx to reset currentVideoPlayRef. This function will reset ref if uploading

Why limit this to the uploading case? (or what's the difference between those cases) I see you mentioned this earlier

But after opening attachment modal, we can't control player because ref is reset. So I'm going to reset currentVideoPlayerRef only if attachment video is local file(it means you are trying to uploading.)

Can you elaborate on this one please?

[serhii1030]

@s77rt
When we're closing attachment modal below function is called.
We can see pauseVideo() function here.
Console error is occurred when running this function because videoplayer object is already unmounted.
In other cases(when running uploaded video), this is not happening because we are using sharedref.
So I reset ref only if we're uploading.

App/src/components/VideoPlayerContexts/PlaybackContext.tsx

Lines 41 to 49 in 0995d40

	const updateCurrentlyPlayingURL = useCallback(
	(url: string | null) => {
	if (currentlyPlayingURL && url !== currentlyPlayingURL) {
	pauseVideo();
	}
	setCurrentlyPlayingURL(url);
	},
	[currentlyPlayingURL, pauseVideo],
	);

[s77rt]

@KMichel1030 Okay, checking this more it looks more generic than being related to the attachment modal only. Here is what we should do instead: On VideoPlayer unmounts, if shouldUseSharedVideoElement is false, then reset the ref.

PS: New test case:

1. Upload video
2. Play that video
3. Delete the video
4. Hover on another video and make sure no errors appear

Screen.Recording.2024-04-11.at.10.09.45.AM.mov

[serhii1030]

@s77rt
Proposal updated!

test.mp4

[s77rt]

@KMichel1030 Can you please test the case where we remove the other video that is not playing? The ref in this case shouldn't reset

[serhii1030]

@s77rt
Yes, you are right. We have to reset ref only if video player is currently playing.
Proposal updated!

test.mp4

[s77rt]

@KMichel1030

We have to reset ref only if video player is currently playing

You mean only if the video player is active (not necessary playing) right? By active I mean that the ref set in the context is the same ref for the video instance that's being removed.

Test case confirming the above: If we open the upload attachment, stop the video and close the modal, the ref should reset.

[serhii1030]

@s77rt
I've updated proposal with this.

useLayoutEffect(() => {
        return () => {
            if(!shouldUseSharedVideoElement && (isUploading || videoPlayerRef.current === currentVideoPlayerRef.current)) {
                currentVideoPlayerRef.current = null;
            }
        }
    }, []);

[s77rt]

@KMichel1030 Overall this looks good to me 👍. Let's discuss the details in the PR.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

The BZ member will need to manually hire KMichel1030 for the Contributor role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[blimpich]

@KMichel1030 please follow these steps so we can hire you. And welcome! 🙂

[serhii1030]

Hi, @abekkala
It seems regression period has been ended.
Could you please let me know how can I get paid for this issue?

[abekkala]

Yes - this was Deployed to Production on April 23

Fix: @KMichel1030 [$250] - can you send your upwork profile link? I can't seem to find you.
PR Review: @s77rt [$250] - paid and contract ended

[serhii1030]

@abekkala
https://www.upwork.com/freelancers/~019b26cf011ef5d91b
I've added contributor details here.
Thanks.

[abekkala]

@KMichel1030 OFFER

[abekkala]

@KMichel1030 payment sent and contract ended - thank you! celebrate 🎉

[serhii1030]

@abekkala
Thank you very much.