tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
185 stars 42 forks source link

"Promise rejected error" on opening game #17

Closed HiEv closed 4 years ago

HiEv commented 4 years ago

SugarCube v2.30.0 and tested in Opera.

Whenever you first open the HTML in a published story it throws a:

Promise rejected error: DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD

This causes the game to be paused in the debugger if you have the inspection window open.

To fix this, the code should avoid trying to play anything if there's nothing to play.

Thanks.

tmedwards commented 4 years ago

What version of Opera? The most recent version, which it claims is 65.0.3467.78, yields the following:

Uncaught (in promise) DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD

The (in promise) bit there is key. Exceptions thrown within a Promise do not stop code execution as the Promise itself consumes the exception—that's the only way to make exceptions .then()/.catch()-able. If you do not explicitly handle the error via .then() or .catch(), the only thing that happens is that a message is logged about the exception—because the Promise already consumed it.

A quick test using SugarCube v2.30.0 proves this. Even with the developer tools open the only thing that happens is the logged message.

To fix this, the code should avoid trying to play anything if there's nothing to play.

I don't know what this means. Playback is only attempted if somewhere in the game the author attempted to play a track.

HiEv commented 4 years ago

Yes, Opera v65.0.3467.78, though it happened in earlier versions of Opera as well.

Are you not seeing the problem? Just create a blank story in Twine, publish it, open it in Opera, open the "Developer Tools" window, and then hit the "Restart" button on the UI bar. The code will pause when it hits that promise rejection.

Playback is only attempted if somewhere in the game the author attempted to play a track.

That doesn't appear to be the case. For example, opening that blank project in Firefox (v71.0) I see the warning, "Autoplay is only allowed when approved by the user, the site is activated by the user, or media is muted." So it looks like it's attempting to play something, otherwise you wouldn't get that error, correct?

That said, I'm not seeing anything in the current versions of Chrome or Vivaldi, so the problem appears to be browser-specific.

tmedwards commented 4 years ago

No, I'm not seeing the problem and yes I attempted to reproduce it—as I noted in my last reply.

Are you perhaps running Opera with non-default settings/extensions/whatever?

Firefox's warning

Firefox's warning is logged because of the Has.audioPromise test, which does call <HTMLAudioElement>.play(). Apparently, it doesn't like the no-op function passed to .catch()—or that's my guess until I look into it more. I can probably silence it simply by muting the HTMLAudioElement instance. Regardless, it's only a warning and does not actually affect anything.

The test itself is structured the way it is specifically because of Blink-/WebKit-based browsers, which still seem happy with it as-is.

tmedwards commented 4 years ago

Hmm. Firefox logs the warning regardless of volume/mute state. Setting one or both of .volume = 0 and .muted = true does not affect it. Even tried adding a valid track, just to see if that was part of the conundrum, and still no dice. Firefox is simply being pissy here for zero reason. How wonderful.

HiEv commented 4 years ago

I tested on a computer with a fresh install of Opera and it didn't pause.

I looked into it further, and it depends on whether you have "Pause on exceptions" turned on (it's a button under the "Sources" tab that looks like a stop sign, but with a sideways equals sign on it). Chrome will act the same if you have that option turned on.

I had that turned on to make it easier to find and debug exceptions.

So there's an exception that's always being thrown at that point.

tmedwards commented 4 years ago

Yes, and that's not the only place. Exceptions can be thrown in feature testing, hence the explicit handling of the exceptions possibly thrown there via try/catch—and, in this specific case, also the <Promise>.catch(). All of the Has and Web Storage tests are like that—the latter are notorious for thowing.

Examples of the technique from elsewhere: 1, 2. The point being that this type of test is not unusual and is found all over the place: Modernizr, jQuery, es-shims, etc.

Specifically to this case, the Has.audioPromise test. Blink's debugger is pausing on exceptions generated within a Promise even with its Pause on caught exceptions checkbox disabled and the exception being handled by the <Promise>.catch() method. That doesn't seem like good behavior to me—or Firefox, apparently, since its debugger doesn't do that when set to pause on exceptions; or Edge's, but it doesn't currently seem to care either way.

Removing the <Promise>.catch() results in both Firefox and Blink-browsers logging errors.

Firefox simply logs the play error, which is only half the issue:

NotAllowedError: The play method is not allowed by the user agent or the platform in the current context, possibly because the user denied permission.

Blink-browsers log the play error, in addition to the reason the error is being logged in the first place—emphasis added:

Uncaught (in promise) DOMException: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD

Restoring the <Promise>.catch(), the only browser that so much as coughs by default is Firefox and it simply logs a warning—a unnecessary warning, but that's not unusual for Firefox.

TL;DR:

Blink's debugger seems to be ignoring its own Pause on caught exceptions setting for Promises—pausing even if it's disabled. I don't know why, but it's bad behavior.