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

Remove invaild catches from replayer #4038

Closed
ma2ciek opened this issue Apr 4, 2017 · 4 comments · Fixed by ckeditor/ckeditor5-engine#908
Closed

Remove invaild catches from replayer #4038

ma2ciek opened this issue Apr 4, 2017 · 4 comments · Fixed by ckeditor/ckeditor5-engine#908
Assignees
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 4, 2017

These catches should be changed in order to provide some correct errors.

@ma2ciek ma2ciek self-assigned this Apr 4, 2017
@Reinmar
Copy link
Member

Reinmar commented Apr 5, 2017

They are rather unnecessary – if the function return a promise it's usually enough when the caller handles errors.

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 5, 2017

They were designed to handle errors from the applyNextDelta method as I didn't notice that this may conflict with the errors coming from the engine

@Reinmar
Copy link
Member

Reinmar commented Apr 5, 2017

OK, so this goes a bit beyond my knowledge of this code and its application. So, my only point remains that there should be no silent catches. The rest is up to you :)

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 5, 2017

I changed the applyNextDelta method and now it resolves also when there is no delta to replay. Thus, the methods that uses applyNextDelta can correctly capture the engine errors, e.g. the play method (test).

scofalik referenced this issue in ckeditor/ckeditor5-engine Apr 6, 2017
Fix: Removed invalid promise catches from `dev-utils.DeltaReplayer`. Closes #906.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants