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

First approach to fix issue with dbg/run start before PSES running #1436

Merged
merged 1 commit into from
Jul 16, 2018

Conversation

rkeithhill
Copy link
Contributor

PR Summary

Fix #1433

One minor issue is that if you abort (return null|undefined) from
resolveDebugConfiguration, VSCode "helpfully" opens your launch.json
file for you. Actually, that is quite annoying. I found an issue and on
this and voted it up - microsoft/vscode#54213

Also fix logic for "attach" error. We only need to test if
OS != Windows. If on Windows, PS Core supports attach. And tweaked the
error message wording to make more clear.

If the user attempts to start a dgb or "run with out debugging" session
before PSES is running, a NullRefEx occurs in PSES. Ideally, we would
wait in the resolveDebugConfiguration method for PSES to finish
initializing with a max wait time of say 10 seconds. Unfortunately,
"sleep" in a loop in JavaScript is not so easy. AFAIT requires a
significant rewrite of the method using setTimeout(). Not sure it is
worth it, unless someone more knowledgable in JS can find an easy
way to do the poll (for sessionstatus)/sleep loop.

BTW there is probably a fix we need to make in PSES to check if
SynchronizationContext is not null before we try to use it.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

Fix #1433

One minor issue is that if you abort (return null|undefined) from
resolveDebugConfiguration, VSCode "helpfully" opens your launch.json
file for you.  Actually, that is quite annoying. I found an issue and on
this and voted it up - microsoft/vscode#54213

Also fix logic for "attach" error.  We only need to test if
OS != Windows.  If on Windows, PS Core supports attach. And tweaked the
error message wording to make more clear.

If the user attempts to start a dgb or "run with out debugging" session
before PSES is running, a NullRefEx occurs in PSES. Ideally, we would
wait in the resolveDebugConfiguration method for PSES to finish
initializing with a max wait time of say 10 seconds.  Unfortunately,
"sleep" in a loop in JavaScript is not so easy.  AFAIT requires a
significant rewrite of the method using setTimeout().  Not sure it is
worth it, unless someone more knowledgable in JS can find an easy
way to do the poll (for sessionstatus)/sleep loop.

BTW there is probably a fix we need to make in PSES to check if
SynchronizationContext is not null before we try to use it.
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Jul 16, 2018

I take it you're able to start debugging, get the warning, then wait, then start debugging again, and it works?

@rkeithhill
Copy link
Contributor Author

Yes.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I think that just firing the warning instead of waiting until the the server is ready probably makes sense -- although if we really don't like it we can always work on that as a longer term solution.

@rkeithhill rkeithhill merged commit bb545ab into master Jul 16, 2018
@rkeithhill rkeithhill deleted the user/hillr/prevent-dbg-start-pses-not-running branch July 16, 2018 23:44
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

Successfully merging this pull request may close these issues.

Crashed on script start
3 participants