Bug 83893 - when demarshalling an object method call fails, the error is weird
Summary: when demarshalling an object method call fails, the error is weird
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2014-09-15 14:59 UTC by Simon McVittie
Modified: 2018-08-22 22:02 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Add a regression test for fd.o #83530 (11.75 KB, patch)
2014-09-15 15:01 UTC, Simon McVittie
Details | Splinter Review
gerror_to_dbus_error_message: add support for remaining members of DBusGError (2.60 KB, patch)
2014-09-15 15:03 UTC, Simon McVittie
Details | Splinter Review
invoke_object_method: when demarshalling fails, use the right error (2.56 KB, patch)
2014-09-15 15:05 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2014-09-15 14:59:44 UTC
I wrote a regression test for Bug #80557. That test revealed that when something goes wrong while we demarshal a message (either excessive variant recursion depth or an unsupported type), we report it as o.fd.DBus.GLib.ErrorError.

I think this is unintended: I would expect ErrorError to be reserved for "assertion failure: C code is doing wrong things with GError". However, it's been like this since 2005.
Comment 1 Simon McVittie 2014-09-15 15:01:07 UTC
Created attachment 106325 [details] [review]
Add a regression test for fd.o #83530

---

You will notice that there is a #if 0...#else...#endif block for the InvalidSignature error that Alban added, because it isn't actually emitted correctly.
Comment 2 Simon McVittie 2014-09-15 15:03:32 UTC
Created attachment 106326 [details] [review]
gerror_to_dbus_error_message: add support for remaining  members of DBusGError

Nobody updated this when we added these 15 new error codes.

---

If anyone is in any doubt about dbus-glib's maintenance status... this has been missing since at least 2008, and before then, dbus-glib didn't even have a properly stable ABI for errors. Such quality.
Comment 3 Simon McVittie 2014-09-15 15:05:02 UTC
Created attachment 106327 [details] [review]
invoke_object_method: when demarshalling fails, use the  right error

This replaces the inappropriate org.freedesktop.DBus.GLib.ErrorError
with the intended error codes, affecting two tests.

---

With this, the InvalidSignature error that Alban added is actually thrown, as intended.

NoMemory doesn't seem great for "your variants are too recursive"... but whatever, it's been raising ErrorError all this time, NoMemory isn't really any worse.
Comment 4 Simon McVittie 2014-09-15 15:05:58 UTC
(In reply to comment #1)
> Add a regression test for fd.o #83530

... no, this is a regression test for fd.o #80557. I'll correct this when I push.
Comment 5 GitLab Migration User 2018-08-22 22:02:48 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus-glib/issues/1.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.