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

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

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")

I can of course remove the word `Error` in the second case. My question
then is: will `(car err)` always be the word "error"? Or may there be
another content?

> > It seems harmless, I am just uncertain if we should prefer %S or %s
> > to
> > format the error message.
>
> `%s` to print `err` or `(cdr err)` would be wrong, since `%s` is for
> use
> with strings rather than lists.  IOW, IMO, it should be either
>
>     ...%S" ... err)
>
> or
>
>     ...%s" ... (error-message-string err))
>
> where the first is a bit more "debugging/developer" friendly and the
> second
> is a bit more "user" friendly.

In my tests there seems to be no difference in the output between %s
and %S. I would presume doing `(message "%s" '(a b))` would result in
error as the param isn't a string, but it works.