{"thread":{"id":"33913","subject":"[PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails","startedAt":"2013-05-23T20:05:03Z","lastAt":"2013-05-29T15:26:53Z","messageCount":6,"participants":["Matthieu Moy","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"218305","messageId":"1369339503-12426-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"33913","inReplyTo":null,"subject":"[PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-05-23T20:05:03Z","receivedAt":"2013-05-23T20:05:03Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"My use-case is an invalid SSL certificate. Pulling from the wiki with a\nrecent version of libwww-perl fails, and git-remote-mediawiki gave no\nclue about the reason. Give the mediawiki API detailed error message, and\nsince it is not so informative, hint the user about an invalid SSL\ncertificate on https:// urls.\n\nSigned-off-by: Matthieu Moy \n---\n contrib/mw-to-git/git-remote-mediawiki.perl | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\nindex 9c14c1f..5b6e833 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki.perl\n+++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n@@ -292,7 +292,13 @@ sub get_mw_all_pages {\n \tif (!defined($mw_pages)) {\n \t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n \t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n-\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\tif ($url =~ /^https/) {\n+\t\t print STDERR \"fatal: make sure '$url/api.php' is a valid page\\n\";\n+\t\t print STDERR \"fatal: and the SSL certificate is correct.\\n\";\n+\t\t} else {\n+\t\t print STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\t}\n+\t\tprint STDERR \"error: \" . $mediawiki->{error}->{details} . \"\\n\";\n \t\texit 1;\n \t}\n \tforeach my $page (@{$mw_pages}) {\n-- \n1.8.3.rc3.8.g5e49f30\n"},{"id":"218691","messageId":"20130528180727.GB21210@sigill.intra.peff.net","threadId":"33913","inReplyTo":"1369339503-12426-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-28T18:07:27Z","receivedAt":"2013-05-28T18:07:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote:\n\n> My use-case is an invalid SSL certificate. Pulling from the wiki with a\n> recent version of libwww-perl fails, and git-remote-mediawiki gave no\n> clue about the reason. Give the mediawiki API detailed error message, and\n> since it is not so informative, hint the user about an invalid SSL\n> certificate on https:// urls.\n\nThis is definitely an improvement, but it seems like it just the tip of\nthe iceberg.\n\nThe call in get_mw_tracked_categories already uses die() with the MW\nerror code and detailed string. Good. The call you fix here prints a\nguess to stderr and exits 1, and your patch improves that. But the call\nin get_mw_first_pages does the same broken thing, and is not fixed.\nDitto for get_all_mediafiles. Other calls do not even seem to error\ncheck the result at all, and assume the result is not undef (which I\nsuspect would produce a perl runtime error).\n\nI wonder if we can do something like:\n\n our $mw_operation;\n $mediawiki->{config}->{on_error} = sub {\n my $err = \"fatal: \";\n if (defined $mw_operation) {\n $err .= \"unable to $mw_operation: \";\n }\n err .= $mediawiki->{error}->{details};\n die \"$err\\n\";\n };\n\nand then callers do not have to worry about error-checking at all. If\nthey want a nicer human-readable indication of where the error occured,\nthey can do:\n\n local $mw_operation = \"get the list of remote wiki pages\";\n my $mw_pages = $mediawiki->list(...);\n\n-Peff\n"},{"id":"218806","messageId":"vpqzjvej8p4.fsf@anie.imag.fr","threadId":"33913","inReplyTo":"20130528180727.GB21210@sigill.intra.peff.net","subject":"Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-05-29T12:01:59Z","receivedAt":"2013-05-29T12:01:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote:\n>\n>> My use-case is an invalid SSL certificate. Pulling from the wiki with a\n>> recent version of libwww-perl fails, and git-remote-mediawiki gave no\n>> clue about the reason. Give the mediawiki API detailed error message, and\n>> since it is not so informative, hint the user about an invalid SSL\n>> certificate on https:// urls.\n>\n> This is definitely an improvement, but it seems like it just the tip of\n> the iceberg.\n\nRight.\n\n> I wonder if we can do something like:\n>\n> our $mw_operation;\n> $mediawiki->{config}->{on_error} = sub {\n> [...]\n> die \"$err\\n\";\n> };\n\nProbably, but that would hardcode the fact that mediawiki errors are\nfatal, while in an ideal world, some errors should be recoverable, and\nsome would require some cleanups before die-ing.\n\nAlso, an error during the first mediawiki operation should not\nnecessarily have the same diagnosis hint as the others: if I just\ndid a successfull querry, and the next fails, it can hardly be an SSL\ncertificate error.\n\nI'll send a v2 that covers a bit more (at least, push and pull with an\ninvalid certificate both give the message).\n\nMore work is needed to get a real good error management, but I don't\nhave time for that now.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"218807","messageId":"1369829189-27881-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"33913","inReplyTo":"vpqzjvej8p4.fsf@anie.imag.fr","subject":"[PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-05-29T12:06:29Z","receivedAt":"2013-05-29T12:06:29Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"My use-case is an invalid SSL certificate. Pulling from the wiki with a\nrecent version of libwww-perl fails, and git-remote-mediawiki gave no\nclue about the reason. Give the mediawiki API detailed error message, and\nsince it is not so informative, hint the user about an invalid SSL\ncertificate on https:// urls.\n\nSigned-off-by: Matthieu Moy \n---\n contrib/mw-to-git/git-remote-mediawiki.perl | 26 ++++++++++++++++++--------\n 1 file changed, 18 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\nindex 9c14c1f..f0c348c 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki.perl\n+++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n@@ -238,6 +238,22 @@ sub mw_connect_maybe {\n \t}\n }\n \n+sub fatal_mw_error {\n+\tmy $action = shift;\n+\tprint STDERR \"fatal: could not $action.\\n\";\n+\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\tif ($url =~ /^https/) {\n+\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page\\n\";\n+\t\tprint STDERR \"fatal: and the SSL certificate is correct.\\n\";\n+\t} else {\n+\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t}\n+\tprint STDERR \"fatal: (error \" .\n+\t $mediawiki->{error}->{code} . ': ' .\n+\t $mediawiki->{error}->{details} . \")\\n\";\n+\texit 1;\n+}\n+\n ## Functions for listing pages on the remote wiki\n sub get_mw_tracked_pages {\n \tmy $pages = shift;\n@@ -290,10 +306,7 @@ sub get_mw_all_pages {\n \t\taplimit => 'max'\n \t});\n \tif (!defined($mw_pages)) {\n-\t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n-\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n-\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n-\t\texit 1;\n+\t\tfatal_mw_error(\"get the list of wiki pages\");\n \t}\n \tforeach my $page (@{$mw_pages}) {\n \t\t$pages->{$page->{title}} = $page;\n@@ -316,10 +329,7 @@ sub get_mw_first_pages {\n \t\ttitles => $titles,\n \t});\n \tif (!defined($mw_pages)) {\n-\t\tprint STDERR \"fatal: could not query the list of wiki pages.\\n\";\n-\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n-\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n-\t\texit 1;\n+\t\tfatal_mw_error(\"query the list of wiki pages\");\n \t}\n \twhile (my ($id, $page) = each(%{$mw_pages->{query}->{pages}})) {\n \t\tif ($id < 0) {\n-- \n1.8.3.rc3.8.g5e49f30\n"},{"id":"218845","messageId":"20130529152216.GA9072@sigill.intra.peff.net","threadId":"33913","inReplyTo":"vpqzjvej8p4.fsf@anie.imag.fr","subject":"Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-29T15:22:16Z","receivedAt":"2013-05-29T15:22:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 29, 2013 at 02:01:59PM +0200, Matthieu Moy wrote:\n\n> > I wonder if we can do something like:\n> >\n> > our $mw_operation;\n> > $mediawiki->{config}->{on_error} = sub {\n> > [...]\n> > die \"$err\\n\";\n> > };\n> \n> Probably, but that would hardcode the fact that mediawiki errors are\n> fatal, while in an ideal world, some errors should be recoverable, and\n> some would require some cleanups before die-ing.\n\nFortunately this is perl, not C. We can catch and re-throw die\nexceptions like:\n\n my $mw_pages = eval { $mediawiki->list(...) };\n if (!$mw_pages) {\n # possibly continue to something else, or even...\n clean_up();\n die; # propagate $@\n }\n\nbut it would require checking all of the call-sites. Another alternative\nwould be a wrapper function that each caller could opt into. But I\nsuspect the norm will be to die, so the exception model should make the\ncode cleaner.\n\n> Also, an error during the first mediawiki operation should not\n> necessarily have the same diagnosis hint as the others: if I just\n> did a successfull querry, and the next fails, it can hardly be an SSL\n> certificate error.\n\nYeah, my error template was just a sketch; I didn't look too carefully\nat all of the callers, but the concept should be extensible.\n\n> I'll send a v2 that covers a bit more (at least, push and pull with an\n> invalid certificate both give the message).\n\nThanks.\n\n-Peff\n"},{"id":"218846","messageId":"20130529152653.GB9072@sigill.intra.peff.net","threadId":"33913","inReplyTo":"1369829189-27881-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-29T15:26:53Z","receivedAt":"2013-05-29T15:26:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 29, 2013 at 02:06:29PM +0200, Matthieu Moy wrote:\n\n> My use-case is an invalid SSL certificate. Pulling from the wiki with a\n> recent version of libwww-perl fails, and git-remote-mediawiki gave no\n> clue about the reason. Give the mediawiki API detailed error message, and\n> since it is not so informative, hint the user about an invalid SSL\n> certificate on https:// urls.\n> \n> Signed-off-by: Matthieu Moy \n\nThanks, this looks like a reasonable stopping point for now.\n\nThere are still some calls that do not check the result of the mediawiki\ncalls at all, and I think they'll probably cause a perl error when they\nfail (for dereferencing undef). But these ones are the initial contact,\nso they should be the common ones to fail. Refactoring the whole error\nhandling is a bit larger task and can wait until somebody is interested.\n\n-Peff\n"}]}