After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 792277 - valac should not allow DBus interface methods not throwing IOError
valac should not allow DBus interface methods not throwing IOError
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: D-Bus
unspecified
Other All
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2018-01-06 17:33 UTC by lukas
Modified: 2018-02-12 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError (1.35 KB, patch)
2018-02-10 14:33 UTC, Michael 'Mickey' Lauer
none Details | Review
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError (1.60 KB, patch)
2018-02-11 11:26 UTC, Michael 'Mickey' Lauer
none Details | Review
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError (1.74 KB, patch)
2018-02-11 16:19 UTC, Michael 'Mickey' Lauer
none Details | Review
vala: DBus methods are recommended to throw at least DBusError or IOError (2.98 KB, patch)
2018-02-11 17:54 UTC, Rico Tzschichholz
none Details | Review
vala: Issue a warning on DBus methods which are not throwing an Error (3.11 KB, patch)
2018-02-11 20:10 UTC, Rico Tzschichholz
committed Details | Review

Description lukas 2018-01-06 17:33:31 UTC
This code

[DBus (name = "com.github.lmberg.example")]
public interface Test : Object {
    public abstract bool ping ();
}

compiles without any complaints.

However, DBus interface methods should not be declared without "throws IOError". Otherwise, no error would be thrown if Test.ping() could not be found on the bus, for example. (GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod)

From https://wiki.gnome.org/Projects/Vala/DBusServerSample : "The methods of the client interface must be defined with throws IOError. "
Comment 1 Michael 'Mickey' Lauer 2018-01-28 19:16:52 UTC
Thanks for your bug report. I agree that it is unfortunate, since it hides potential CRITICAL run-time errors making the program abort unexpectedly.

We can't make it a compile error immediately though, since that would break a lot of existing code unnecessarily.

I propose to emit a warning like "WARNING: Declaring a DBus method without throwing at least IOError is deprecated." and changing this to a hard error eventually.

I'll work on a patch (if no-one else wants to).
Comment 2 Michael 'Mickey' Lauer 2018-02-10 14:33:51 UTC
Created attachment 368211 [details] [review]
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError

Here's a patch that might do the right thing. Would anyone of the core developers please review and give feedback?
Comment 3 Michael 'Mickey' Lauer 2018-02-11 11:26:30 UTC
Created attachment 368229 [details] [review]
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError

2nd try, this time the logic is put into vala, as opposed to the codegen. Should also adhere to code style guide now.
Comment 4 Michael 'Mickey' Lauer 2018-02-11 16:19:35 UTC
Created attachment 368231 [details] [review]
Report a deprecation warning when a DBus method does not throw at least DBusError and IOError
Comment 5 Rico Tzschichholz 2018-02-11 17:54:02 UTC
Created attachment 368233 [details] [review]
vala: DBus methods are recommended to throw at least DBusError or IOError

This issues a warning now and will be turned into an error at some point.
Comment 6 Michael 'Mickey' Lauer 2018-02-11 19:02:19 UTC
Thanks for polishing my patch! I have one question though… you changed the semantics into a "IOError _or_ DBusError". To my knowledge, Vala error domains do not form a hierarchy, or do they? I was thinking that IOError covers other type of errors than DBusError, hence a proper DBus method would be required to throw both.
Comment 7 Rico Tzschichholz 2018-02-11 20:10:02 UTC
Created attachment 368235 [details] [review]
vala: Issue a warning on DBus methods which are not throwing an Error

It is recommended to throw "GLib.Error" or "GLib.DBusError, GLib.IOError".
This will be turned into an error at some point.
Comment 8 Rico Tzschichholz 2018-02-12 09:16:49 UTC
Attachment 368235 [details] pushed as 07c8865 - vala: Issue a warning on DBus methods which are not throwing an Error