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

onstart is called after dataAvailable fired #171

Closed
rotem925 opened this issue Dec 2, 2018 · 8 comments
Closed

onstart is called after dataAvailable fired #171

rotem925 opened this issue Dec 2, 2018 · 8 comments

Comments

@rotem925
Copy link

rotem925 commented Dec 2, 2018

Hi Chris,

I see a new issue that wasn't there on version 5.0
Seems that the dataAvailable callback is called before the onstart is fire.
I'm referencing this old issue again #132

@chris-rudmin
Copy link
Owner

@rotem925 Regression :( I'll give it a look.

@chris-rudmin
Copy link
Owner

@rotem925 I was unable to write a good unit test for my fix. Could you test it on your side and validate?

@rotem925
Copy link
Author

@chris-rudmin Nope, that didn't fix that.
I think I have a fix though, dunno if that ok with you
See attached screenshot
screen shot 2018-12-10 at 17 27 46

@chris-rudmin
Copy link
Owner

@rotem925 I made a second attempt to fix this (new commits to the same branch). Care to try it out?

@rotem925
Copy link
Author

@chris-rudmin No success here also
Is my fix not enough?

@chris-rudmin
Copy link
Owner

@rotem925 I aprreciate your suggestion, but your fix doesn't make sense to me. You are running onstart (which I assume returns a promise in your implementation) in parallel with initSourceNode and initWorker. It is not "correct" in the sense that likely the recording has not started yet when it is called.

The last fix I made only generates the header pages after onstart is called, so I don't understand how the onstart callback could be called after pages are generated.

@rotem925
Copy link
Author

rotem925 commented Dec 11, 2018 via email

@chris-rudmin
Copy link
Owner

@rotem925 I tested and validated my fix works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants