Bug 23369

Summary: [0.11] Make better use of telepathy-spec 0.17.27 errors
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: tp-glibAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/even-better-errors
Whiteboard: review+
i915 platform: i915 features:
Bug Depends on: 24116, 27967    
Bug Blocks:    

Description Simon McVittie 2009-08-17 11:01:50 UTC
I have an insufficiently-tested branch, wip-errors, which I'll finish at some point (currently, the code exists but the tests fail).
Comment 1 Simon McVittie 2009-09-23 06:56:25 UTC
Not suitable for changing in a stable branch, in any case.
Comment 2 Simon McVittie 2010-04-27 08:33:34 UTC
I finished off the branch as smcv/better-errors, and added another, 
smcv/even-better-errors. The former is relatively safe, the latter less so.

I can divide the better-errors branch up into 3 parts (TpBaseConnection, TpConnection, TpChannel Group) and get them reviewed separately, if one turns out to be controversial. Branches on the way in a moment (currently rebasing, since there were changes in the tests that are likely to conflict).

TpBaseConnection
================

When #TpBaseConnectionClass.start_connecting fails, any GError it yields is turned into a Connection_Status_Reason. I just added some more cases. This should be uncontroversial.

TpConnection
============

Old:
* Network_Error, "...ConnectionFailed" -> TP_ERROR_CONNECTION_FAILED
* Network_Error, "com.example.NoNetworkCoverage" -> TP_DBUS_ERROR_REMOTE_ERROR

New:
* Network_Error, "...ConnectionFailed" -> TP_ERROR_CONNECTION_FAILED
* Network_Error, "com.example.NoNetworkCoverage" -> TP_ERROR_NETWORK_ERROR

This also adds a function tp_connection_get_detailed_error(), which gets the ConnectionError arguments (if that signal was emitted) or a D-Bus error string chosen based on the invalidation reason (otherwise), and is necessary for Bug #21120.

TpChannel Group (in better-errors)
==================================

This changes the TpProxy::invalidated error if a D-Bus error string is seen in the "error" detail of MembersChangedDetailed. Previously, we used the Channel_Group_Change_Reason to construct an error in TP_ERRORS_REMOVED_FROM_GROUP; now, we use the "error", if recognised. If the "error" is not recognised, we still fall back to the Channel_Group_Change_Reason.

Old:
* Error, "...ConnectionFailed" -> domain=TP_ERRORS_REMOVED_FROM_GROUP, code=TP_CHANNEL_GROUP_CHANGE_REASON_ERROR
* Error, "com.example.NoNetworkCoverage" -> domain=TP_ERRORS_REMOVED_FROM_GROUP, code=TP_CHANNEL_GROUP_CHANGE_REASON_ERROR

New:
* Error, "...ConnectionFailed" -> TP_ERROR_CONNECTION_FAILED
* Error, "com.example.NoNetworkCoverage" -> domain=TP_ERRORS_REMOVED_FROM_GROUP, code=TP_CHANNEL_GROUP_CHANGE_REASON_ERROR

TpChannel Group (in even-better-errors)
=======================================

This additionally changes the behaviour of TpChannel's TpProxy::invalidated when there isn't an "error", or it isn't recognised. Instead of encoding the Channel_Group_Change_Reason into TP_ERRORS_REMOVED_FROM_GROUP, it's now encoded into TP_ERRORS, whenever possible, which makes TP_ERRORS_REMOVED_FROM_GROUP mostly obsolete.

As a special case, Channel_Group_Change_Reason_Error turns into the NotAvailable error code (I could change this if someone has a better idea).

Old:
* Kicked -> domain=TP_ERRORS_REMOVED_FROM_GROUP, code=TP_CHANNEL_GROUP_CHANGE_REASON_KICKED
* Error, "com.example.NoNetworkCoverage" -> domain=TP_ERRORS_REMOVED_FROM_GROUP, code=TP_CHANNEL_GROUP_CHANGE_REASON_ERROR

New:
* Kicked -> TP_ERROR_CHANNEL_KICKED
* Error, "com.example.NoNetworkCoverage" -> TP_ERROR_NOT_AVAILABLE
Comment 3 Simon McVittie 2010-04-27 08:34:41 UTC
Sorry, I meant Bug #21200 rather than Bug #21120.
Comment 5 Simon McVittie 2010-04-27 09:06:33 UTC
It seems (from `git grep TP_ERROR`) that this won't impact Empathy at all, since Empathy does virtually nothing with errors. :'-(
Comment 6 Guillaume Desmottes 2010-04-28 00:50:10 UTC
better-errors looks good to me.
Didn't review even-better-errors yet.
Comment 7 Simon McVittie 2010-04-28 10:08:14 UTC
I've merged most of better-errors; because even-better-errors hasn't been reviewed yet, I omitted the changes to TpChannel (I think if we're going to make changes to TpChannel's error behaviour, they should all land at once, so I'd like even-better-errors either accepted or rejected before merging the rest).
Comment 8 Guillaume Desmottes 2010-05-04 06:36:35 UTC
Looks good to me.
Comment 9 Simon McVittie 2010-05-04 09:51:15 UTC
I've updated the branch with some text for NEWS. Merge is blocked by Bug #27967.
Comment 10 Simon McVittie 2010-05-04 10:00:17 UTC
r+ from wjt for the NEWS, merging.
Comment 11 Simon McVittie 2010-05-04 10:15:04 UTC
Fixed in git for 0.11.5

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.