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

feat: add an error code for SessionClosedError #729

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

wyfo
Copy link
Contributor

@wyfo wyfo commented Oct 1, 2024

Resolves #727

Copy link

github-actions bot commented Oct 1, 2024

PR missing one of the required labels: {'documentation', 'new feature', 'internal', 'enhancement', 'dependencies', 'bug', 'breaking-change'}

@wyfo wyfo added the enhancement New feature or request label Oct 1, 2024
src/result.rs Outdated
@@ -25,6 +25,7 @@ pub const Z_ENETWORK: z_result_t = -4;
pub const Z_ENULL: z_result_t = -5;
pub const Z_EUNAVAILABLE: z_result_t = -6;
pub const Z_EDESERIALIZE: z_result_t = -7;
pub const Z_SESSION_CLOSED: z_result_t = -8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Z_ESESSION_CLOSED maybe, to make in the same style as all other errors ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/publisher.rs Outdated
result::Z_OK
match put.wait() {
Ok(_) => result::Z_OK,
Err(e) if e.downcast_ref::<SessionClosedError>().is_some() => result::Z_SESSION_CLOSED,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need some specific tracing message here and in other places ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think so, because it's kind of included in the error kind. So the user can choose to ignore it without cluttering the logs.

@milyin milyin enabled auto-merge (squash) October 1, 2024 10:07
@milyin milyin merged commit 4cc81ef into eclipse-zenoh:main Oct 1, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We need a dedicated return code when session is closed
3 participants