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

debug console shows REPL results and console.log output in wrong order #79196

Closed
weinand opened this issue Aug 15, 2019 · 6 comments

Comments

@weinand
Copy link
Member

commented Aug 15, 2019

evaluating var i = 0; console.log(++i); ++i; in the REPL shows output in the wrong order.

@weinand weinand added bug debug labels Aug 15, 2019
@weinand weinand added this to the On Deck milestone Aug 15, 2019
@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

I can reprouce this nicely.

Debug adapter is returning results in the correct order the issue is on the vscode side.
The issue is that we have an object that represents the user input and the evaluation output together. Due to these two being tied together it is not possible for any output to appear between them.
Now to think about it I am not sure what was the design decision to go with this "double object" approach. But it seems like it does not make a lot of sense.
Due to this we are also hitting a limitation that if an evaluate takes too long we do not show the input.

So the fix for this would be to use seperate objects in the model for input and evaluate output.

@pavelfeldman

This comment has been minimized.

Copy link

commented Aug 20, 2019

Top-level await is another use case for this. When we say await fetch('/'), source expression is not visible until we get the response. Async operations are meant to take long and having synchronous console logs in between of evaluation and result is also expected.

@isidorn isidorn modified the milestones: On Deck, September 2019 Aug 26, 2019
dgozman added a commit to dgozman/vscode that referenced this issue Aug 30, 2019
REPL now has two elements: one for the evaluation text added immediately,
and another for the evaluation result once available.
dgozman added a commit to dgozman/vscode that referenced this issue Sep 3, 2019
REPL now has two elements: one for the evaluation text added immediately,
and another for the evaluation result once available.
@dgozman

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@isidorn Is someone actively working on this? I see it is added to the milestone and don't want to interfere. I have a rough version of the fix for this, and wonder whether it makes sense to contribute a PR? Thanks a lot!

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

@dgozman I planned to look into this next week and I have not started coding yet.
However if you have a PR which contributes a nice fix I would be very interested to see it. So maybe you can create it with what you have and we can start a discussion there.
I wonder if you tackled this like I suggested to split the "double objects" in the REPL model in two, or you found another fix. Anyways we can discuss in the PR - thanks a lot!

dgozman added a commit to dgozman/vscode that referenced this issue Sep 7, 2019
REPL now has two elements: one for the evaluation text added immediately (ReplEvaluationInput),
and another for the evaluation result once available (ReplEvaluationResult).
@isidorn isidorn closed this in 3bcaff8 Sep 10, 2019
isidorn added a commit that referenced this issue Sep 10, 2019
Separate REPL evaluation from it's result; fixes #79196
@dgozman

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

I found that there is still an issue here related to issue #33822: simple evaluation result is shown before a complex console.log output due to async processing of DAP output. I will look into fixing this.

Repro: evaluating console.log({foo: 'bar'}); 123 shows 123 followed by Object {foo: "bar"}.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

@dgozman yeah, the cause is exactly what you mentioned, the output event comes asyncly.
Not sure how to easily fix this without introducing some order to the incoming messages. Looking forward to seeing your approach.

dgozman added a commit to dgozman/vscode that referenced this issue Sep 19, 2019
; microsoft#79196

Two fixes:
- handle each DAP message in a separate task to ensure that all
  microtasks run in between;
- append repl elements in sequential order by chaining their async
  processing.
@octref octref added the verified label Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.