Skip to content

fix: improve exit codes for agent/agentssh and cli/ssh #10850

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 24, 2023
Merged

fix: improve exit codes for agent/agentssh and cli/ssh #10850

merged 4 commits into from
Nov 24, 2023

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Nov 23, 2023 •
edited
Loading

This PR improves the exit codes of agent/agentssh and cli/ssh (non-stdio).

In cases where the process is killed via signal, agentssh returned -1 which was translated into a 32-bit uint max (4294967295) over the wire. What this logically translates to is 255 (uint8(-1)).

Consequently, cli/ssh was also returning nonsense exit codes of 0/1 only and masking the true exit code. This is now fixed.

~/Work/Code/coder mafredri/fix-ssh-exit*
❯ coder ssh work; echo $?

mafredri@work ~
❯ exit 2
Encountered an error running "coder ssh"
session ended:
    github.com/coder/coder/v2/cli.(*RootCmd).ssh.func1
        /home/runner/actions-runner/_work/coder/coder/cli/ssh.go:388
  - Process exited with status 2
1

Compared to:

~/Work/Code/coder mafredri/fix-ssh-exit*
❯ ./coder ssh work; echo $?

mafredri@work ~
❯ exit 2
2

@mafredri mafredri self-assigned this Nov 23, 2023
@mafredri mafredri force-pushed the mafredri/fix-ssh-exit branch from 659eb5c to ce50540 Compare November 24, 2023 11:52
@mafredri mafredri requested review from johnstcn and mtojek November 24, 2023 12:13
@mafredri mafredri merged commit 61be4df into main Nov 24, 2023
@mafredri mafredri deleted the mafredri/fix-ssh-exit branch November 24, 2023 12:35
@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@johnstcn johnstcn johnstcn approved these changes

@mtojek mtojek mtojek approved these changes

Assignees

@mafredri mafredri

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mafredri @johnstcn @mtojek