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

Report errors from mosh-server startup to the user #1232

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cgull
Copy link
Member

@cgull cgull commented Oct 27, 2022

This pull request mostly fixes a longtime irritant in Mosh-- the lack of error reporting when the ssh command to start mosh-server goes sideways. It does this by rearranging some code in mosh-server so the session information is sent before the server forks, and rearranging some code code in mosh.pl so that the session information is captured in the parent, not the child. When those two things are done, then mosh-server can report some errors before it forks off into the sunset, and mosh.pl can usefully capture the exitstatus of the ssh command. There's a new test to check some of that too.

fixes #1042
partially addresses #1005
and would have prevented countless questions on IRC.

Early exit from a block is clearer.
This allows its use for error messages in the parent-- used in the
next commit.
I think this will help with the papercut of "Did not find mosh server
startup message".  This message confuses people; often the real
problem is that the command to start the remote mosh-server failed
somehow.

Fixes mobile-shell#1042, partially resolves mobile-shell#1005.

Also relevant for mobile-shell#1042 and countless questions on IRC.
@cgull
Copy link
Member Author

cgull commented Nov 3, 2022

#1211 is a recent example of an issue where I think this change will help.

@cgull
Copy link
Member Author

cgull commented Nov 4, 2022

I misremembered, there's no mosh-server changes here-- I'd made those changes in 2016, in 4ad131a.

@achernya
Copy link
Collaborator

Does this PR obsolete #1097 now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot connect to host
2 participants