Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented Mar 7, 2024

Right now, if spawning the remote command fails for some reason, we get a message like the following in the trace output:

pure SSH protocol connection failed: Unable to negotiate version with remote side (unable to read capabilities): EOF

However, all we know is that we failed to connect to the remote git-lfs-transfer process. That could be for any number of reasons: the remote program doesn't exist or is not in PATH, the user has misconfigured something locally or remotely, or any number of other problems. We won't know unless we log the output, so let's do that so we can get more information.

Right now, if spawning the remote command fails for some reason, we get
a message like the following in the trace output:

  pure SSH protocol connection failed: Unable to negotiate version with remote side (unable to read capabilities): EOF

However, all we know is that we failed to connect to the remote
`git-lfs-transfer` process.  That could be for any number of reasons:
the remote program doesn't exist or is not in `PATH`, the user has
misconfigured something locally or remotely, or any number of other
problems.  We won't know unless we log the output, so let's do that so
we can get more information.
@bk2204 bk2204 force-pushed the improved-pure-ssh-failure-logging branch from 60f8360 to 5dbbf13 Compare March 12, 2024 13:21
@bk2204 bk2204 marked this pull request as ready for review March 12, 2024 14:17
@bk2204 bk2204 requested a review from a team as a code owner March 12, 2024 14:17
Copy link
Member

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@bk2204 bk2204 merged commit eda4fd7 into git-lfs:main Mar 12, 2024
@bk2204 bk2204 deleted the improved-pure-ssh-failure-logging branch March 12, 2024 18:47
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 22, 2024
With PR git-lfs#4446 we defined a new SSHTransfer structure and set of methods
on that structure in our ssh/connection.go source file, and used the
name "tr" for the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file, in order to format and output an error message from the
startConnection() function.  As it happens this function is not one of
the methods of the SSHTransfer structure, and so there was no conflict
with the methods' "tr" receiver variables.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr variable,
which we can not do with the current name of the "tr" receiver variable,
as it masks the "tr" package name within the method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 23, 2024
With PR git-lfs#4446 we defined a new SSHTransfer structure and set of methods
on that structure in our ssh/connection.go source file, and used the
name "tr" for the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file, in order to format and output an error message from the
startConnection() function.  As it happens this function is not one of
the methods of the SSHTransfer structure, and so there was no conflict
with the methods' "tr" receiver variables.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr variable,
which we can not do with the current name of the "tr" receiver variable,
as it masks the "tr" package name within the method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
With PR git-lfs#4446 we defined a new SSHTransfer structure and set of methods
on that structure in our ssh/connection.go source file, and used the
name "tr" for the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file, in order to format and output an error message from the
startConnection() function.  As it happens this function is not one of
the methods of the SSHTransfer structure, and so there was no conflict
with the methods' "tr" receiver variables.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr variable,
which we can not do with the current name of the "tr" receiver variable,
as it masks the "tr" package name within the method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 1, 2024
With PR git-lfs#4446 we defined a new SSHTransfer structure and set of methods
on that structure in our ssh/connection.go source file, and used the
name "tr" for the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file, in order to format and output an error message from the
startConnection() function.  As it happens this function is not one of
the methods of the SSHTransfer structure, and so there was no conflict
with the methods' "tr" receiver variables.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr variable,
which we can not do with the current name of the "tr" receiver variable,
as it masks the "tr" package name within the method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 3, 2024
With PR git-lfs#4446 we defined a new SSHTransfer structure and set of methods
on that structure in our ssh/connection.go source file, and used the
name "tr" for the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file, in order to format and output an error message from the
startConnection() function.  As it happens this function is not one of
the methods of the SSHTransfer structure, and so there was no conflict
with the methods' "tr" receiver variables.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr variable,
which we can not do with the current name of the "tr" receiver variable,
as it masks the "tr" package name within the method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 4, 2024
In PR git-lfs#4446 we defined a new SSHTransfer structure and a set of methods
for it in our ssh/connection.go source file, and used the name "tr" for
the receiver variables of those methods.

Later, in commit 5dbbf13 of PR git-lfs#5674,
we imported our "tr" message translation package into the same source
file in order to format and localize an error message generated by the
startConnection() function.  Because this function is not one of the
methods of the SSHTransfer structure there was no conflict with the
"tr" receiver variables of those methods.

However, in subsequent commits in this PR we expect to revise one of
the SSHTransfer structure's methods to output an error message, and
will want to use the Get() method of the "tr" package's global Tr
variable.  We are not able to do this with the current name of the
"tr" receiver variable as it masks the "tr" package name within the
method's scope.

Therefore we now rename all our "tr" receiver variables to "st" in the
ssh/connection.go source file, so as to avoid any namespace conflicts
with our "tr" package.
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.

2 participants