Bug with debug autoAttach in dev containers

[Sti2nd]

• VSCode Version: 1.59.1 (user setup)
• Local OS Version: Windows_NT x64 10.0.19042
• Remote Extension/Connection Type: Dev container. Docker container which is again running on WSL
• Logs:

Steps to Reproduce:

Get a devcontainer with Node, for example https://github.com/microsoft/vscode-dev-containers/tree/v0.187.0/containers/javascript-node-postgres

1. Add the following line to the settings field in devcontainer.json: "debug.javascript.autoAttachFilter": "smart".
2. Rebuild container
3. Running node in the terminal now gives the following error:

internal/modules/cjs/loader.js:905
  throw err;
  ^

Error: Cannot find module '/home/node/.vscode-server/data/User/workspaceStorage/82385960831832aaa7dc0f79ab321f88/ms-vscode.js-debug/bootloader.js'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
    at Function.Module._load (internal/modules/cjs/loader.js:746:27)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1244:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:467:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:70:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}

There are several similar bugs at the VS Code repository, just linking one for now: microsoft/vscode#98778
Feel free to move this to the VS Code repository if it fits better there.

To make auto attach working I can do Debug: Toggle Auto Attach from the command palette and toggle it off and on again, but that defeats the purpose of having it set as a default setting in the devcontainer.json 😊

[chrmarti]

@connor4312 Rebuilding probably deletes the initial copy. Is there a way to trigger copying that again?

[connor4312]

Only if we can activate the extension somehow. I think rebuilding should clear the terminal environment variable collection since anything put in there may not longer be applicable or reference correctly in the rebuilt container @Tyriar

[Tyriar]

@connor4312 instead of referencing workspaceStorage, can you make a bootloader entry point in the (stable) extension install directory instead and have that do the detection/installation?

[connor4312]

Copying to the workspace folder was done in microsoft/vscode-js-debug@3056cb8 since the path of the extension folder for the nightly extension changes daily. We could use the install folder for only the non-nightly extension and just accept that this bug will happen using nightly, which is kind of spooky but would solve the issue.

[chrmarti]

@Tyriar It seems NODE_OPTIONS is reapplied to the new terminals after the container is rebuilt? Is there a way for the terminals to start as if this is a new remote not seen before (the container is new)? Maybe Remote-Containers could provide a hint when the container was rebuilt.

[Tyriar]

Right now only the extension can modify its own EnvironmentVariableCollection, the problem here is that to vscore core the workspace looks like the same thing but it's actually not. Open to ideas on an API or something for remote containers to do this.

[chrmarti]

The resolver could return a machine id that changes when the container changes. I guess, you could then flush all EnvironmentVariableCollection for that URI, not sure if that's more than you would want to flush though.

Since this is the only issue that has come up in this regard, I'm also open to add a simple workaround to Remote-Containers.

Thinking about this, I wonder: How does the extension make sure the environment variable is set when it does not activate on/after startup in a new workspace? Should it activate "onStartupFinished"?

[connor4312]

How does the extension make sure the environment variable is set when it does not activate on/after startup in a new workspace? Should it activate "onStartupFinished"?

Its environment variables are persisted over reloads without requiring activation to be set in a given window -- since the user can create terminals before activation happens.

[Tyriar]

The resolver could return a machine id that changes when the container changes

Does such a concept exist yet? Does core know that machines may differ for remotes?

[chrmarti]

How does the extension make sure the environment variable is set when it does not activate on/after startup in a new workspace? Should it activate "onStartupFinished"?

Its environment variables are persisted over reloads without requiring activation to be set in a given window -- since the user can create terminals before activation happens.

I see the built-in debug-auto-launch activates on * and knows about the stable and nightly extensions. Could that also update the environment variables?

The resolver could return a machine id that changes when the container changes

Does such a concept exist yet? Does core know that machines may differ for remotes?

That would be new.

[Tyriar]

I guess what we want then is a StorageScope of WorkspaceMachine here that uses that concept?

https://github.com/microsoft/vscode/blob/b8205f2aa4d55c09b97e7ec2ba16aa45b90adf2a/src/vs/workbench/contrib/terminal/common/environmentVariableService.ts#L36-L36

eg. it could use the regular workspace storage key when machine isn't set, but if the remote extension sets it, it could be <regular>#<machineid>

[chrmarti]

There is also StorageTarget.MACHINE, maybe that already covers this. What's a bit special is that the 'machine' is new after a rebuild, but it is usually still very similar to the previous one, so I guess there might also be advantages to keeping some of the StorageTarget.MACHINE values. (E.g., if the open editors are stored using that.)

@connor4312 Before we dive in more on a general solution, couldn't the following check also check if the bootloader.js exists and if not activate js-debug like it already does for the other cases: https://github.com/microsoft/vscode/blob/775423265668153122d48760de12c3a46a616313/extensions/debug-auto-launch/src/extension.ts#L370

[connor4312]

The environment variables are necessarily set by js-debug, since it needs to know the location of the bootloader and apply settings to the environment variables. The API doesn't allow extensions to see each others' environment variable collections, so debug-auto-launch can't read it. A hack around would be to create a hidden terminal and echo back the variables in some way on each startup, but that seems very inefficient...

[Tyriar]

Another solution would be for variable resolution to support fetching an extension's directory, like ${extensionDir:ms-vscode.js-debug}/bootloader.js

[connor4312]

For this we need to start consuming extensions in the variable resolver. If we want to do this globally, then I think we'd want to:

• Split the IExtensionService interface into a 'read-only' part, and a mutable part. The read-only part contains information about extensions and will need to be replicated into the extension host for the resolver to deal with.
• Rework some things so that that can be injected when creating the variable resolver and refactor some methods. I've mostly done this part locally already.

The alternative would be saying that we only support this in the limited scope of terminal environment variables, which would let us create a more focused change to inject extension information only when resolving terminal environment variables. Thoughts @sandy081?

[sandy081]

@alexdima owns IExtensionService.

[alexdima]

I like the suggestion from @Tyriar for supporting ${extensionDir:extensionId} in the variable resolving.

@connor4312 sorry, I'm not sure I follow your comment. AFAIK variable resolution for remote terminals is done on the renderer process, because remote terminals are launched from the server process. I'm not sure I understand where the need to split the IExtensionService comes from.

[connor4312]

It looks like extension resolution only happens in the extension host as part of the IDebugAdapterFactory which is eventually implemented by ExtHostDebugService.$substituteVariables. This was implemented by Andre a couple years ago as part of making debugging independent from Node APIs. If we can narrow that or avoid doing so, that means we would not have to replicate the logic in the extension host and therefore the "split" (to make a subset of IExtensionService information accessible in the extension host) would be unnecessary. I'll follow up with him.