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 687503 - Improve error message »Failed to refresh folders«
Improve error message »Failed to refresh folders«
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
3.6.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
Depends on:
Blocks: 502515
 
 
Reported: 2012-11-03 13:38 UTC by Paul Menzel
Modified: 2012-12-03 17:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] mail/mail-send-recv.c: Elaborate error message when refreshing folders (Closes: 687503) (2.49 KB, patch)
2012-11-03 13:45 UTC, Paul Menzel
none Details | Review
[PATCH v2] Bug 687503 – Add user, server and folder to error message when refreshing a folder fails (3.42 KB, patch)
2012-11-04 13:34 UTC, Paul Menzel
needs-work Details | Review
eds patch (1.35 KB, patch)
2012-12-03 17:31 UTC, Milan Crha
committed Details | Review
evo patch (1.67 KB, patch)
2012-12-03 17:32 UTC, Milan Crha
committed Details | Review

Description Paul Menzel 2012-11-03 13:38:09 UTC
Even with Milan’s fix in version 3.6.0

    commit 6b255c9309d642d59c14e809ab9a7144d93409f3
    Author: Milan Crha <mcrha@redhat.com>
    Date:   Fri Aug 17 13:58:57 2012 +0200

        Include folder uri in "Failed to refresh folder" runtime warning

the current error message printed to the terminal

    (evolution:11858): evolution-mail-WARNING **: Failed to refresh folder: folder://1344563600.xxxx.32%40myhostname/Drafts: Server-Verbindung unerwartet getrennt:I/O operation timed out

is not very useful to the user. Changing it to something like

    evolution-mail-WARNING **: Failed to refresh folder Drafts (joey@mail.example.net): Server-Verbindung unerwartet getrennt:I/O operation

is preferable.
Comment 1 Paul Menzel 2012-11-03 13:45:17 UTC
Created attachment 227963 [details] [review]
[PATCH] mail/mail-send-recv.c: Elaborate error message when refreshing folders (Closes: 687503)

Here is a draft patch.

More or less copied from Evolution Data Server.

    camel/providers/imap/camel-imap-command.c

1. I need to improve the commit message.

2. Also I do not know which of the variables need to be freed.

3. I think, the folder information can also be given in the error string returned by Evolution Data Server. Unfortunately for some reason, I might have edited the wrong part and turned to Evolution then.

So leave Milan change as is and let the error string contain the user and server information.
Comment 2 Paul Menzel 2012-11-04 13:34:11 UTC
Created attachment 228023 [details] [review]
[PATCH v2] Bug 687503 – Add user, server and folder to error message when refreshing a folder fails

v2: Update changelog

1. If the local folder ID(?) is useful for the user, it should be left in there.
2. What variable need to be freed?
Comment 3 Milan Crha 2012-11-09 09:41:00 UTC
OK, once again, this is just a console message, regular users will not notice it. Messages on console are meant as developer hints, and with certain circumstances any g_warning can lead to application's abort.

For the patch itself:
a) do not read all the variables always, but only when needed, thus just
   before the g_warning() call - you added unnecessary overhead there
b) use
     CamelStore *store = camel_folder_get_parent_store (folder);
     const gchar *account_name = camel_service_get_display_name (
           CAMEL_SERVICE (store));

   This way you get the same name as is shown in the folder tree and there is
   no extra memory allocation involved.
Comment 4 Milan Crha 2012-12-03 17:31:01 UTC
Created attachment 230544 [details] [review]
eds patch

for evolution-data-server;

I updated your patch. I realized I overlooked that the CamelFolder is unreffed before this g_Warning() too, thus I changed that too. With this, the message looks either like:
   Failed to refresh folder 'accountName: Inbox/...': error message
or, if the folder is unavailable:
   Failed to refresh folder 'folder://uis/Inbox/...': error message
where the 'accountName' is the account's name, as shown in the left folder tree.
Comment 5 Milan Crha 2012-12-03 17:32:30 UTC
Created attachment 230545 [details] [review]
evo patch

for evolution; (of course, I'm sorry for my mistake)

I updated your patch. I realized I overlooked that the CamelFolder is unreffed
before this g_Warning() too, thus I changed that too. With this, the message
looks either like:
   Failed to refresh folder 'accountName: Inbox/...': error message
or, if the folder is unavailable:
   Failed to refresh folder 'folder://uis/Inbox/...': error message
where the 'accountName' is the account's name, as shown in the left folder
tree.
Comment 6 Milan Crha 2012-12-03 17:33:29 UTC
Created commit 01a3fd2 in evo master (3.7.3+)