ripe-tech / ripe-sdk

The public Javascript SDK for RIPE Core
https://www.platforme.com
Apache License 2.0
8 stars 4 forks source link

fix: configurator cancel signature error #234

Closed BeeMargarida closed 3 years ago

BeeMargarida commented 3 years ago
- -
Issue Originated from https://github.com/ripe-tech/ripe-twitch-ui/pull/54 (read description)
Dependencies --
Decisions From what I understood, the configuration cancel operation proceeds if the signatures don't match, since this means that there is an update process happening. However, the signature assignment was happening earlier in the update process and, when trying to cancel it would not work, since the signatures would match, even with the _loadFrame and preload happening.

By moving the signature assignment below, the signatures will only match when all promises are finished, and the cancel operation will successfully be performed when the _preload action is in the process (by running the _finalize).

The reasoning behind this came from a problem where the configurator was canceled in the middle of an update. This caused the SDK to get stuck when updating its children in the code below https://github.com/ripe-tech/ripe-sdk/blob/767dfd77d3635b45e611462bfb95950d96bc205b/src/js/base/ripe.js#L1294 It seems the update gets stuck on the preloadPromise of the configurator, which was not canceled by cancel called before https://github.com/ripe-tech/ripe-sdk/blob/767dfd77d3635b45e611462bfb95950d96bc205b/src/js/visual/configurator-prc.js#L269 The configuration cancelation never passed by the _finalize step, canceling all the render operations, even when it was defined.

With this fix, after several tries on my part by spamming the "customization" part of the twitch journey, I couldn't not reproduce the error. @3rdvision also tested and only reproduced it with "ridiculous" spamming. In the last gifs there are examples of this spamming, and it works. I don't know if this fixes the problem.
Animated GIF Problem:
change_stuff_bug
Now:
fix-sdk-journey
spam-sdk-fixed spam-sdk-bug
gcandal commented 3 years ago

@joamag we'll have to take a careful look at this together with @BeeMargarida

@BeeMargarida in the meantime, if you got any free time, please assess if it's feasible to use the pre_update/update/post_update events to disable the ripe-twitch navigation buttons to avoid bumping into this issue.

joamag commented 3 years ago

I believe this fix needs to be improved to address the double nature of the signature, prevent double loading, and prevent delayed canceling. We need to keep signature and create a signatureLoaded value.

joamag commented 3 years ago

@BeeMargarida @gcandal I've changed the solution (not tested at all) but in theory, it should work

joamag commented 3 years ago

@BeeMargarida can you please test it extensively as you did with the other solution?

BeeMargarida commented 3 years ago

@BeeMargarida can you please test it extensively as you did with the other solution?

I seems to be working, even in the more stressing situations :+1:

joamag commented 3 years ago

@BeeMargarida thanks!