* [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
@ 2013-05-23 20:05 Matthieu Moy
2013-05-28 18:07 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2013-05-23 20:05 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
My use-case is an invalid SSL certificate. Pulling from the wiki with a
recent version of libwww-perl fails, and git-remote-mediawiki gave no
clue about the reason. Give the mediawiki API detailed error message, and
since it is not so informative, hint the user about an invalid SSL
certificate on https:// urls.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
contrib/mw-to-git/git-remote-mediawiki.perl | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9c14c1f..5b6e833 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -292,7 +292,13 @@ sub get_mw_all_pages {
if (!defined($mw_pages)) {
print STDERR "fatal: could not get the list of wiki pages.\n";
print STDERR "fatal: '$url' does not appear to be a mediawiki\n";
- print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
+ if ($url =~ /^https/) {
+ print STDERR "fatal: make sure '$url/api.php' is a valid page\n";
+ print STDERR "fatal: and the SSL certificate is correct.\n";
+ } else {
+ print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
+ }
+ print STDERR "error: " . $mediawiki->{error}->{details} . "\n";
exit 1;
}
foreach my $page (@{$mw_pages}) {
--
1.8.3.rc3.8.g5e49f30
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
2013-05-23 20:05 [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails Matthieu Moy
@ 2013-05-28 18:07 ` Jeff King
2013-05-29 12:01 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2013-05-28 18:07 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote:
> My use-case is an invalid SSL certificate. Pulling from the wiki with a
> recent version of libwww-perl fails, and git-remote-mediawiki gave no
> clue about the reason. Give the mediawiki API detailed error message, and
> since it is not so informative, hint the user about an invalid SSL
> certificate on https:// urls.
This is definitely an improvement, but it seems like it just the tip of
the iceberg.
The call in get_mw_tracked_categories already uses die() with the MW
error code and detailed string. Good. The call you fix here prints a
guess to stderr and exits 1, and your patch improves that. But the call
in get_mw_first_pages does the same broken thing, and is not fixed.
Ditto for get_all_mediafiles. Other calls do not even seem to error
check the result at all, and assume the result is not undef (which I
suspect would produce a perl runtime error).
I wonder if we can do something like:
our $mw_operation;
$mediawiki->{config}->{on_error} = sub {
my $err = "fatal: ";
if (defined $mw_operation) {
$err .= "unable to $mw_operation: ";
}
err .= $mediawiki->{error}->{details};
die "$err\n";
};
and then callers do not have to worry about error-checking at all. If
they want a nicer human-readable indication of where the error occured,
they can do:
local $mw_operation = "get the list of remote wiki pages";
my $mw_pages = $mediawiki->list(...);
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
2013-05-28 18:07 ` Jeff King
@ 2013-05-29 12:01 ` Matthieu Moy
2013-05-29 12:06 ` [PATCH v2] " Matthieu Moy
2013-05-29 15:22 ` [PATCH] " Jeff King
0 siblings, 2 replies; 6+ messages in thread
From: Matthieu Moy @ 2013-05-29 12:01 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
Jeff King <peff@peff.net> writes:
> On Thu, May 23, 2013 at 10:05:03PM +0200, Matthieu Moy wrote:
>
>> My use-case is an invalid SSL certificate. Pulling from the wiki with a
>> recent version of libwww-perl fails, and git-remote-mediawiki gave no
>> clue about the reason. Give the mediawiki API detailed error message, and
>> since it is not so informative, hint the user about an invalid SSL
>> certificate on https:// urls.
>
> This is definitely an improvement, but it seems like it just the tip of
> the iceberg.
Right.
> I wonder if we can do something like:
>
> our $mw_operation;
> $mediawiki->{config}->{on_error} = sub {
> [...]
> die "$err\n";
> };
Probably, but that would hardcode the fact that mediawiki errors are
fatal, while in an ideal world, some errors should be recoverable, and
some would require some cleanups before die-ing.
Also, an error during the first mediawiki operation should not
necessarily have the same diagnosis hint as the others: if I just
did a successfull querry, and the next fails, it can hardly be an SSL
certificate error.
I'll send a v2 that covers a bit more (at least, push and pull with an
invalid certificate both give the message).
More work is needed to get a real good error management, but I don't
have time for that now.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails
2013-05-29 12:01 ` Matthieu Moy
@ 2013-05-29 12:06 ` Matthieu Moy
2013-05-29 15:26 ` Jeff King
2013-05-29 15:22 ` [PATCH] " Jeff King
1 sibling, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2013-05-29 12:06 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
My use-case is an invalid SSL certificate. Pulling from the wiki with a
recent version of libwww-perl fails, and git-remote-mediawiki gave no
clue about the reason. Give the mediawiki API detailed error message, and
since it is not so informative, hint the user about an invalid SSL
certificate on https:// urls.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
contrib/mw-to-git/git-remote-mediawiki.perl | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9c14c1f..f0c348c 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -238,6 +238,22 @@ sub mw_connect_maybe {
}
}
+sub fatal_mw_error {
+ my $action = shift;
+ print STDERR "fatal: could not $action.\n";
+ print STDERR "fatal: '$url' does not appear to be a mediawiki\n";
+ if ($url =~ /^https/) {
+ print STDERR "fatal: make sure '$url/api.php' is a valid page\n";
+ print STDERR "fatal: and the SSL certificate is correct.\n";
+ } else {
+ print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
+ }
+ print STDERR "fatal: (error " .
+ $mediawiki->{error}->{code} . ': ' .
+ $mediawiki->{error}->{details} . ")\n";
+ exit 1;
+}
+
## Functions for listing pages on the remote wiki
sub get_mw_tracked_pages {
my $pages = shift;
@@ -290,10 +306,7 @@ sub get_mw_all_pages {
aplimit => 'max'
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not get the list of wiki pages.\n";
- print STDERR "fatal: '$url' does not appear to be a mediawiki\n";
- print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
- exit 1;
+ fatal_mw_error("get the list of wiki pages");
}
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
@@ -316,10 +329,7 @@ sub get_mw_first_pages {
titles => $titles,
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not query the list of wiki pages.\n";
- print STDERR "fatal: '$url' does not appear to be a mediawiki\n";
- print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
- exit 1;
+ fatal_mw_error("query the list of wiki pages");
}
while (my ($id, $page) = each(%{$mw_pages->{query}->{pages}})) {
if ($id < 0) {
--
1.8.3.rc3.8.g5e49f30
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] git-remote-mediawiki: better error message when HTTP(S) access fails
2013-05-29 12:06 ` [PATCH v2] " Matthieu Moy
@ 2013-05-29 15:26 ` Jeff King
0 siblings, 0 replies; 6+ messages in thread
From: Jeff King @ 2013-05-29 15:26 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
On Wed, May 29, 2013 at 02:06:29PM +0200, Matthieu Moy wrote:
> My use-case is an invalid SSL certificate. Pulling from the wiki with a
> recent version of libwww-perl fails, and git-remote-mediawiki gave no
> clue about the reason. Give the mediawiki API detailed error message, and
> since it is not so informative, hint the user about an invalid SSL
> certificate on https:// urls.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Thanks, this looks like a reasonable stopping point for now.
There are still some calls that do not check the result of the mediawiki
calls at all, and I think they'll probably cause a perl error when they
fail (for dereferencing undef). But these ones are the initial contact,
so they should be the common ones to fail. Refactoring the whole error
handling is a bit larger task and can wait until somebody is interested.
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
2013-05-29 12:01 ` Matthieu Moy
2013-05-29 12:06 ` [PATCH v2] " Matthieu Moy
@ 2013-05-29 15:22 ` Jeff King
1 sibling, 0 replies; 6+ messages in thread
From: Jeff King @ 2013-05-29 15:22 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
On Wed, May 29, 2013 at 02:01:59PM +0200, Matthieu Moy wrote:
> > I wonder if we can do something like:
> >
> > our $mw_operation;
> > $mediawiki->{config}->{on_error} = sub {
> > [...]
> > die "$err\n";
> > };
>
> Probably, but that would hardcode the fact that mediawiki errors are
> fatal, while in an ideal world, some errors should be recoverable, and
> some would require some cleanups before die-ing.
Fortunately this is perl, not C. We can catch and re-throw die
exceptions like:
my $mw_pages = eval { $mediawiki->list(...) };
if (!$mw_pages) {
# possibly continue to something else, or even...
clean_up();
die; # propagate $@
}
but it would require checking all of the call-sites. Another alternative
would be a wrapper function that each caller could opt into. But I
suspect the norm will be to die, so the exception model should make the
code cleaner.
> Also, an error during the first mediawiki operation should not
> necessarily have the same diagnosis hint as the others: if I just
> did a successfull querry, and the next fails, it can hardly be an SSL
> certificate error.
Yeah, my error template was just a sketch; I didn't look too carefully
at all of the callers, but the concept should be extensible.
> I'll send a v2 that covers a bit more (at least, push and pull with an
> invalid certificate both give the message).
Thanks.
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-29 15:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-23 20:05 [PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails Matthieu Moy
2013-05-28 18:07 ` Jeff King
2013-05-29 12:01 ` Matthieu Moy
2013-05-29 12:06 ` [PATCH v2] " Matthieu Moy
2013-05-29 15:26 ` Jeff King
2013-05-29 15:22 ` [PATCH] " Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).