Include container status to IncorrectStatus error messaging#3411
Open
CarloQuick wants to merge 2 commits intoyouki-dev:mainfrom
Open
Include container status to IncorrectStatus error messaging#3411CarloQuick wants to merge 2 commits intoyouki-dev:mainfrom
CarloQuick wants to merge 2 commits intoyouki-dev:mainfrom
Conversation
utam0k
added
the
kind/cleanup
utam0k
approved these changes
Feb 17, 2026
YJDoc2
reviewed
Feb 17, 2026
crates/libcontainer/src/error.rs
Outdated
| pub enum LibcontainerError { | ||
| #[error("failed to perform operation due to incorrect container status")] | ||
| IncorrectStatus, | ||
| #[error("failed to perform operation due to the container's status: `{0}`")] |
Collaborator
There was a problem hiding this comment.
nit:
Suggested change
| #[error("failed to perform operation due to the container's status: `{0}`")] | |
| #[error("failed to perform operation due to unexpected container status: `{0}`")] |
My reasoning is that the error message should have some indication that this status was not correct/expected. Previously, we used to say incorrect status.
However, not a blocker, but like to hear opinion.
Author
There was a problem hiding this comment.
This makes sense.
The operation being requested is wrong for the status.
What are your thoughts the following?
failed operation due to incompatible container status: `Running`
Author
There was a problem hiding this comment.
@utam0k are you referring to suggested wording, correct?
Author
YJDoc2
approved these changes
Feb 17, 2026
Signed-off-by: Carlo Quick <98628846+CarloQuick@users.noreply.github.com>
CarloQuick
force-pushed
the
feat/incorrect-status-error
branch
from
b4977be to
aaa00d9
Compare
Signed-off-by: Carlo Quick <98628846+CarloQuick@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
IncorrectStatuswas using generic error message, so I includedContainerStatusto theIncorrectStatuserror variant.// Before Caused by: failed to perform operation due to incorrect container status // After Caused by: failed to perform operation due to the container's status: `Running`Type of Change
Testing
Added new unit tests
Added new integration tests
Ran existing test suite
Tested manually (please provide steps)
Added unit test:
test_libcontainer_error_msgto verify correct formatting and display of each container status in the error messaging.Ran existing test suite: Ran
libcontainertests locally withcargo test -p libcontainer,tty::tests::test_setup_consoleandtty::tests::test_verify_ptmx_handle_with_real_ptyfail locally. (These appear to be pre-existing environment-specific failures (devpts on WSL2), unrelated to this change.)Tested manually: I created a container, and while running attempted to delete it without the force flag. The displayed error message contained the container's status.
Related Issues
Fixes #3410
Additional Context
n/a