microsoft / vscode-remote-release

Visual Studio Code Remote Development: Open any folder in WSL, in a Docker container, or on a remote machine using SSH and take advantage of VS Code's full feature set.
https://aka.ms/vscode-remote
Other
3.67k stars 289 forks source link

Using forwardPorts gives no indication that a different port was used #2249

Open Chuxel opened 4 years ago

Chuxel commented 4 years ago

I hit a problem where I had port 3000 running locally, and then opened a dev container with "forwardPorts": [3000]. Since the port conflicted, the port was actually mapped to 3001 locally. However, I had no idea, and couldn't figure out why updates weren't taking effect. I finally realized what was going on.

This doesn't happen with appPort because it actually errors if the port is already in use.

Repro:

  1. Spin up something on port 3000 locally (like another docker container)
  2. Create a devcontainer.json that points to "forwardPorts": [3000].

Expected: Notified that a different local port was used, or it errors Actual: No notice, no error

chrmarti commented 4 years ago

I'm not sure we want this to be a notification. The user might not care about all the forwarded ports.

Chuxel commented 4 years ago

@chrmarti Yeah, I understand not wanting to have notification spam, but if they went to the trouble of adding the forwardPorts property, they do care. Only the Azure Functions definitions include it by default - and that's because you do always access the function on the same port.

In many ways, the old erroring behavior was less confusing. However, since there's no way to declare a different port mapping, there needs to be some way to let you know things are not how you configured them in the json file. Given how common it is to use certain ports in multiple apps (3000, 5000, 8080), I could see this easily happening.

Chuxel commented 4 years ago

@chrmarti Forgot to mention - I'm not suggesting always notifying, but rather notifying in this exception case.

chrmarti commented 4 years ago

There is also the team scenario where a user uses a devcontainer.json where they didn't author the forwardPorts themselves and might not care about all ports.

We could handle that with a 'Don't Show Again' button. Maybe writing to a setting, so it is easy to reset and syncs across machines.

sbungartz commented 9 months ago

I just stumbled across this issue and was wondering if this is not solved adequately by the option requireLocalPort (see https://containers.dev/implementors/json_reference/#port-attributes)? This might obviously not have existed back in 2020, but setting it to true will still use a different port but it will open a popup dialog window telling you that it could not use the requeted port. This also allows to just be notified about the ports that have to be fixed for the project at hand and allows other ports to be changed freely without any noise.

Maybe this does not fully solve this issue, but I'll still leave this here since it provides a good solution for us and maybe someone else coming across this issue will find it useful as well.