Bug 39334 - Misreports RequestConnection failing as NetworkError
Summary: Misreports RequestConnection failing as NetworkError
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~smcv/tel...
Whiteboard: r+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-07-18 06:36 UTC by Will Thompson
Modified: 2013-02-13 13:38 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Remove McdAccount::connection-status-changed signal (2.00 KB, patch)
2013-01-09 12:07 UTC, Simon McVittie
Details | Splinter Review
Communicate the detailed error from McdConnection to McdAccount (12.49 KB, patch)
2013-01-09 12:07 UTC, Simon McVittie
Details | Splinter Review

Description Will Thompson 2011-07-18 06:36:13 UTC
If RequestConnection fails, MC disregards the specific error, and just claims a network error occurred:

    if (tperror)
    {
        g_warning ("%s: RequestConnection failed: %s",
                   G_STRFUNC, tperror->message);

        g_signal_emit (connection, signals[CONNECTION_STATUS_CHANGED], 0,
            TP_CONNECTION_STATUS_DISCONNECTED,
            TP_CONNECTION_STATUS_REASON_NETWORK_ERROR, NULL);
        return;
    }

This is not very helpful. My reading of the spec is that accounts with invalid parameters should be marked with Valid: False. I don't think that would be helpful either. ;)

We should probably use the DBus error name returned by RequestConnection—in this case, InvalidArgument. (I've checked Empathy and it currently doesn't do anything useful with InvalidArgument, but that could be changed.) I can't really see any better Connection_Status_Reason than None_Specified. :(

(Obviously, this is a sign that I should add validation to the new “Username” field in Empathy for IRC accounts.)
Comment 1 Simon McVittie 2013-01-09 12:06:56 UTC
Turns out I had a branch for this a few months ago but got distracted by a bee. Here it is...
Comment 2 Simon McVittie 2013-01-09 12:07:20 UTC
Created attachment 72715 [details] [review]
Remove McdAccount::connection-status-changed signal

Nothing listens to it.
Comment 3 Simon McVittie 2013-01-09 12:07:39 UTC
Created attachment 72716 [details] [review]
Communicate the detailed error from McdConnection to  McdAccount

In particular, this propagates the error from RequestConnection failing
correctly, so add a regression test for that.
Comment 4 Guillaume Desmottes 2013-01-09 13:11:02 UTC
++
Comment 5 Simon McVittie 2013-02-13 13:38:18 UTC
Fixed in git for 5.15.0.


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.