Skip to content

fix: sshd handle the error #2026

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

Merged
merged 1 commit into from
Jun 17, 2025
Merged

fix: sshd handle the error #2026

merged 1 commit into from
Jun 17, 2025

Conversation

kemingy
Copy link
Member

@kemingy kemingy commented Jun 16, 2025

Fix the panic when sshd exec with error:

panic: errors.As: *target must be interface or implement error, found *exec.ExitError

goroutine 65 [running]:
github.com/cockroachdb/errors/errutil.As({0x9dea60, 0xc000387860}, {0x90a780, 0xc0003878a0?})
	/home/runner/go/pkg/mod/github.com/cockroachdb/[email protected]/errutil/as.go:49 +0x547
github.com/cockroachdb/errors.As(...)
	/home/runner/go/pkg/mod/github.com/cockroachdb/[email protected]/errutil_api.go:194
github.com/tensorchord/envd/pkg/remote/sshd.getExitStatusFromError({0x9dea60, 0xc000387860})
	/home/runner/work/envd/envd/pkg/remote/sshd/sshd.go:276 +0x59
github.com/tensorchord/envd/pkg/remote/sshd.sendErrAndExit(0xc0001f2070, {0x9e6ed8, 0xc0003ca1a0}, {0x9dea60, 0xc000387860})
	/home/runner/work/envd/envd/pkg/remote/sshd/sshd.go:265 +0x24e
github.com/tensorchord/envd/pkg/remote/sshd.(*Server).connectionHandler(0xc000134f40, {0x9e6ed8, 0xc0003ca1a0})
	/home/runner/work/envd/envd/pkg/remote/sshd/sshd.go:191 +0xaec
github.com/gliderlabs/ssh.(*session).handleRequests.func1()
	/home/runner/go/pkg/mod/github.com/gliderlabs/[email protected]/session.go:261 +0x27
created by github.com/gliderlabs/ssh.(*session).handleRequests in goroutine 42
	/home/runner/go/pkg/mod/github.com/gliderlabs/[email protected]/session.go:260 +0x4c7

cc @gaocegege

@kemingy kemingy requested review from gaocegege and Copilot June 16, 2025 09:03
@kemingy kemingy requested review from aseaday and zwpaper as code owners June 16, 2025 09:03
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fix panic in SSHD error handling and bump IR library versions

  • Rename error parameter and adjust type assertion to prevent panic in sendErrAndExit
  • Update uvVersion from 0.6.5 to 0.7.10
  • Update pixiVersion from 0.45.0 to 0.48.0

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/remote/sshd/sshd.go Rename error param, fix errors.As target type to *exec.ExitError
pkg/lang/ir/v1/uv.go Bump BuildKit UV library version constant
pkg/lang/ir/v1/pixi.go Bump Pixi library version constant
Comments suppressed due to low confidence (4)

pkg/remote/sshd/sshd.go:259

  • [nitpick] The parameter name exitErr is clear, but inner err variables in the write and exit calls shadow it. Consider renaming those inner error variables (e.g. writeErr, exitCallErr) to avoid confusion.
func sendErrAndExit(logger *logrus.Entry, s ssh.Session, exitErr error) {

pkg/remote/sshd/sshd.go:275

  • There are no existing unit tests for the new getExitStatusFromError behavior. Add tests covering: a) non-ExitError errors, b) ExitError with success=true, and c) ExitError with a syscall.WaitStatus fallback.
var exitErr *exec.ExitError

pkg/lang/ir/v1/uv.go:20

  • After bumping uvVersion, remember to update the project changelog or release notes to reflect compatibility changes in BuildKit UV.
uvVersion = "0.7.10"

pkg/lang/ir/v1/pixi.go:26

  • After bumping pixiVersion, ensure any related documentation (changelog, examples) is updated to reference the new Pixi version.
pixiVersion        = "0.48.0"

@gaocegege gaocegege added this pull request to the merge queue Jun 17, 2025
Merged via the queue into tensorchord:main with commit 599a980 Jun 17, 2025
11 checks passed
@kemingy kemingy deleted the sshd_err branch June 17, 2025 02:05
@kemingy
Copy link
Member Author

kemingy commented Jun 18, 2025

I have verified that this fixed the Zed remote error. Now Zed can connect to the envd container.

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