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

Fixes exception when running in process. Closes #949 #950

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

waldekmastykarz
Copy link
Collaborator

Fixes exception when running in process. Closes #949

@waldekmastykarz waldekmastykarz requested a review from a team as a code owner December 2, 2024 11:39
@waldekmastykarz waldekmastykarz added the pr-bugfix Fixes a bug label Dec 2, 2024
@garrytrinder
Copy link
Contributor

First try, with a new installation of proxy. I was asked to update certificate, which I said yes to. At this point, the process got stuck, until I used Ctrl + C to shut down proxy, which also did not unset the HTTP and HTTPS proxies, so had to do that manually.

image

Second try, proxy was started, I was able to Ctrl + C to shut down proxy and it proxies were unset.

image

The output however was different to running proxy normally, the Hotkeys and Close Proxy messages are missing.

image

@waldekmastykarz
Copy link
Collaborator Author

The message about updating the certificate is unfortunate, because when using concurrently, Dev Proxy runs in non-interactive mode, ie. you can't press keys to control it, because it's running in parallel with other processes. This is also why we're not displaying the keys, because Dev Proxy wouldn't handle them.

We could check if we can handle updating the certificate while running inside a parent process, but I wonder if that's not an edge case that we shouldn't worry about.

@garrytrinder
Copy link
Contributor

This is also why we're not displaying the keys, because Dev Proxy wouldn't handle them.

Got it 👍

We could check if we can handle updating the certificate while running inside a parent process, but I wonder if that's not an edge case that we shouldn't worry about.

I agree, however I think we should communicate this to dev somehow, just in case they do get stuck. This could be just a remark in docs.

@waldekmastykarz
Copy link
Collaborator Author

I agree, however I think we should communicate this to dev somehow, just in case they do get stuck. This could be just a remark in docs.

I noticed that concurrently has support for stdin so we could research if there's a way to keep Dev Proxy interactive and produce guidance for use with concurrently, which solves two problems for us 😅

@waldekmastykarz waldekmastykarz merged commit f68c525 into microsoft:main Dec 4, 2024
4 checks passed
@waldekmastykarz waldekmastykarz deleted the fix-keypress branch December 4, 2024 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: exception when starting proxy from npm script
2 participants