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

Error handling when video files don't exist #1537

Closed
jodeleeuw opened this issue Feb 18, 2021 · 2 comments · Fixed by #2359
Closed

Error handling when video files don't exist #1537

jodeleeuw opened this issue Feb 18, 2021 · 2 comments · Fixed by #2359

Comments

@jodeleeuw
Copy link
Member

See #1530 for rationale.

@jodeleeuw jodeleeuw modified the milestones: 7.0, 7.1 Feb 18, 2021
@becky-gilbert becky-gilbert added this to To do in MOSS milestone 3 via automation Feb 22, 2021
@chrisbrickhouse
Copy link
Contributor

The preloadVideo function in core takes an error callback that gets run if the AJAX request for the file results in 404 (see around 2735 in core). The problem (I think, need to debug more) is that if that error callback is undefined, any error response gets thrown out and the program keeps running resulting in the behavior in #1530 since the plugin doesn't know that something is wrong.

The two possible solutions would be to edit core so that these errors bubble up without a callback, or have extensions explicitly provide callbacks for errors. I have a slight preference for the first one. If not handled, it lets the user know that something's wrong rather than silently proceeding (at the very least the console should probably log a warning). For plugins, it allows errors to be handled using try-catch blocks which helps with readability.

Thoughts?

@chrisbrickhouse
Copy link
Contributor

Has this been fixed already? In testing, the console throws warnings whenever resources don't exist, so I can't replicate the behavior in #1530. I think this can be closed unless an experiment-stopping error is better than the warnings already thrown (which I doubt).

@jodeleeuw jodeleeuw linked a pull request Nov 26, 2021 that will close this issue
@jodeleeuw jodeleeuw moved this from To do - would be nice to Done in MOSS milestone 3 Nov 27, 2021
This was referenced Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants