-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
che-code terminal won't start #21081
Comments
@nils-mosbach could you please check if your Workspace is still running when you try to open a terminal |
It is. Pod is running and I can open a terminal session and execute commands using kubectrl. |
Hello @nils-mosbach , I don't know why I miss yesterday's notification
I think creating separate issues is the way to go and I also like the fact that you report problem. Feedback is always welcome About the issue itself, looking at the code, it seems we're in this part of the code then and here the most important part: Could you confirm you're using Windows ? it might explain while I have not seen the issue as I'm not using it. Then in order to fix the issue, it means that probably we have some invalid so it might be interesting to look at env variables in the code's container |
@benoitf No worries, will do. Yes, I'm using Microsoft Edge on Windows. Workspace I've used was https://github.com/che-incubator/che-code.git. Debugging client code is a little messy, since Edge debugger tries to get ts-files from // resolveWithEnvironment executed with
// environment: undefined
// root: {
// "uri": {
// "$mid": 1,
// "fsPath": "\\projects\\che-code",
// "_sep": 1,
// "external": "vscode-remote://company.dev/projects/che-code",
// "path": "/projects/che-code",
// "scheme": "vscode-remote",
// "authority": "company.dev"
// },
// "name": "che-code",
// "index": 0
// }
// value: "/bin/bash"
public resolveWithEnvironment(environment: IProcessEnvironment, root: IWorkspaceFolder | undefined, value: string): string {
return this.recursiveResolve(this.prepareEnv(environment), root ? root.uri : undefined, value);
} Seems that environment should not be empty. This function should fetch the environment: // _resolveProfile executed with
// profile: {
// "profileName": "dev",
// "path": "/bin/bash",
// "isDefault": false,
// "isAutoDetected": false,
// "overrideName": true,
// "color": "f00"
// }
// options: {
// "remoteAuthority": "company.dev",
// "os": 3
// }
// This call is hit multiple times. The second run `options.remoteAuthority` is undefined.
private async _resolveProfile(profile: ITerminalProfile, options: IShellLaunchConfigResolveOptions): Promise<ITerminalProfile> {
if (options.os === OperatingSystem.Windows) { // DEBUGGING: options.os is Linux
// ...
}
// this call seems to return undefined
const env = await this._context.getEnvironment(options.remoteAuthority);
} And container vars Show environment variables
|
ok I guess on my side it's not empty (so anyway we could exclude env var in the entrypoint that are empty to avoid failures on windows) it's defined there |
Seems that this wasn't causing the issue. I created a patch that doesn't set So after removing all empty envs, the error stays the same. I also tried setting a value for |
ok so it would be interesting to know the root error in |
You're right. It actually seems like checks for Windows pass in https://github.com/che-incubator/che-code/blob/7dc338e79032a0fccab3d854aed99164916f398d/code/src/vs/base/common/platform.ts#L77.
Note sure if tricking vs code to linux is a good thing, or if a check would be better. I'll give the check a go, maybe that solves the issue. private prepareEnv(envVariables: IProcessEnvironment): IProcessEnvironment {
// windows env variables are case insensitive
if (isWindows && envVariables) {
const ev: IProcessEnvironment = Object.create(null);
Object.keys(envVariables).forEach(key => {
ev[key.toLowerCase()] = envVariables[key];
});
return ev;
}
return envVariables;
} |
|
thanks for your great analysis. We're missing some methods in our terminal implementation #21080 and I suspect that getEnvironment is one of them so we could add (before a better implementation) there a new case to handle then instead of returning we could return an empty object |
@benoitf I've tested your recommended change and it solves the issue. Thanks! :) Submitted a PR. |
Checked with latest che-code insider build. Works. Closing the issue. |
Describe the bug
The current version of che-code has issues establishing a terminal session in our case.
I'm not sure if that helps, but default terminal is null in the settings.
I know che-code is currently under active devlopment, so feel free to close the issue if this was something that is already known, or work in progress.
@benoitf We'll start migrating our exisiting theia environment to che-code and do some testing. Since you're currently working on this, should I summarize things that pop up in the epic (#20500), create tickets or just wait and test again after the next release? Che-Code is going to be a huge step forward. Thanks a lot for your effort!
Che version
next (development version)
Steps to reproduce
Open a terminal window in che-code.
Expected behavior
Bash should open.
Runtime
Kubernetes (vanilla)
Screenshots
No response
Installation method
chectl/next
Environment
Linux
Eclipse Che Logs
Chrome browser logs:
Container logs
Additional context
No response
The text was updated successfully, but these errors were encountered: