8000 zb: Log errors from GetAll method calll · Issue #1325 · dbus2/zbus · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

zb: Log errors from GetAll method calll #1325

New issue

Have a question about this project? Sign 8000 up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Open
sandercm opened this issue Apr 5, 2025 · 5 comments
Open

zb: Log errors from GetAll method calll #1325

sandercm opened this issue Apr 5, 2025 · 5 comments
Labels
enhancement New feature or request good first issue Good for newcomers zbus Issues/PRs related to zbus crate

Comments

@sandercm
Copy link
sandercm commented Apr 5, 2025 •
edited
Loading

take the following example client

let connection = zbus::Connection::session().await?;
let proxy = zbus::Proxy::new( // <- miss behaving daemon
    &connection,
    "com.example.rs",
    "/example_rs/reference",
    "com.example.referenceA2",
).await?;

let mut stream: PropertyStream<'_, zvariant::Value> = proxy.receive_property_changed("SantasAddress").await;
while let Some(signal) = stream.next().await {
    println!("Received signal: {:?}", signal.get().await);
}

I was testing out some test dbus interfaces and discovered that when you have a daemon that exposes a few properties 4 in my case but 1 of these causes a dbus error when calling Get on that property. The above code will not show an error or do really anything at all. I was able to track down the issue using dbus-monitor where I noticed zbus will do a GetAll call which silently fails and causes the above code to stall forever without any error or notice.
@sandercm sandercm changed the title subscribe_to_property_change doesn't propagate error from GetAll properties receive_property_changed doesn't propagate error from GetAll properties Apr 5, 2025
@zeenix
Copy link
Contributor
zeenix commented Apr 6, 2025

Thanks for reporting this and providing all the details. There isn't much zbus can do here. zbus already provides the option to disable property caching so you can always directly get the properties and then have some of them as fallible. We added support for fallible properties a while back (15203c1). Would that not work?

@sandercm
Copy link
Author
sandercm commented Apr 6, 2025

The issue really is that we have a usecase where we will dynamically start listening for property changes without actually getting the property. I think the main issue here is that it will just silently fail on the call to listen for property changes. Would it be an idea to maybe add something like Tokio did where you can optionally enable tracing logs? If I got an error log for this error it would have been immediately clear what was happening.

@zeenix
Copy link
Contributor
zeenix commented Apr 6, 2025

Would it be an idea to maybe add something like Tokio did where you can optionally enable tracing logs?

We already have tracing and in fact we trace a lot. If all you're asking for is one more log message, I'm fine with that.

@sandercm
Copy link
Author
sandercm commented Apr 7, 2025

Would it be an idea to maybe add something like Tokio did where you can optionally enable tracing logs?

We already have tracing and in fact we trace a lot. If all you're asking for is one more log message, I'm fine with that.

I didn't know that zbus supported tracing that's very nice! I ran my tests again with tracing enabled and I don't see any Error logs. So yeah I think in this case the perfect solution would be if either the creation of the proxy or the stream gives an error. But just a simple Error log would have already saved me a few hours so it might save the next person that runs into it a few hours aswell.

@zeenix
Copy link
Contributor
zeenix commented Apr 8, 2025

I ran my tests again with tracing enabled and I don't see any Error logs.

Should be an easy PR to provide? ;)

@zeenix zeenix changed the title receive_property_changed doesn't propagate error from GetAll properties zb: Log errors from GetAll method calll Apr 8, 2025
@zeenix zeenix added enhancement New feature or request good first issue Good for newcomers zbus Issues/PRs related to zbus crate labels Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request good first issue Good for newcomers zbus Issues/PRs related to zbus crate
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@zeenix @sandercm
0