Subject: Re: bug#75065: Upon archive download failure print the original error

From: Konstantin Kharlamov <[email-masked]>
Date: Thu, 26 Dec 2024 23:37:31 +0300

On Thu, 2024-12-26 at 20:32 +0000, Stefan Kangas wrote:
> Konstantin Kharlamov <[email-masked]> writes:
> 
> > On Thu, 2024-12-26 at 14:17 -0500, Stefan Monnier wrote:
> > > > > >    (dolist (archive package-archives)
> > > > > > -    (condition-case-unless-debug nil
> > > > > > +    (condition-case-unless-debug err
> > > > > >          (package--download-one-archive archive "archive-
> > > > > > contents" async)
> > > > > > -      (error (message "Failed to download `%s' archive."
> > > > > > -               (car archive))))))
> > > > > > +      (error (message "Failed to download `%s' archive.
> > > > > > Error:
> > > > > > %S"
> > > > > > +               (car archive) (cdr err))))))
> > > > > > 
> > > > > Stefan and Philip, is this okay to install?
> > > 
> > > I agree with the idea behind the patch, but printing just `(cdr
> > > err)`
> > > doesn't seem right, it should print the whole of `err`.
> > 
> > The `car` seems to just contain word error. Here's how both
> > compare:
> > 
> > • current patch with `(cdr err)`:
> >     Failed to download ‘melpa’ archive. Error: ("Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> > 
> > • suggested change with `err`:
> >     Failed to download ‘melpa’ archive. Error: (error "Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> 
> The "Error:" part is redundant, so I think it could be shortened to
> something like:
> 
>     Failed to download ‘melpa’ archive: Location melpa.org/packages/
> is
>     not a url nor an absolute file name

Please note that in the above examples everything after the word
`Error: ` is taken from the exception message, which includes the
parentheses and (if I follow Stefan's advice) the downcased word
"error".

The reason in my patch I used `Error:` is because I have no control
over the text that will follow, so I need to make sure it's clear that
what follows is an error message. Hence my question to Stefan above: if
the downcase word `error` will always be there, then removing `Error:`
and replacing `(cdr err)` with `err` will works just as well.