bug#68799: 30.0.50; emacs --fg-daemon fails silently if server-start fails

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: 68799@debbugs.gnu.org,  monnier@iro.umontreal.ca,  jasonr@gnu.org
>> Date: Tue, 13 Feb 2024 13:04:24 -0500
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Using DAEMON_RUNNING would mean any errors while starting the server
>> > will fail to show the explicit error message about that.  That's a
>> > regression in my book.  So I hope we can find a better solution.
>> 
>> Could you be more specific about what would regress in this situation?
>> Preferably with a step by step of something that behaves worse.
>
> I mean this error message:
>
>   (let ((dn (daemonp)))
>     (when dn
>       (when (stringp dn) (setq server-name dn))
>       (server-start)
>       (if server-process
>         (daemon-initialized)
>       (if (stringp dn)
>           (message
>            "Unable to start daemon: Emacs server named %S already running"
>            server-name)
>         (message "Unable to start the daemon.\nAnother instance of Emacs is 
> running the server, either as daemon or interactively.\nYou can use 
> emacsclient to connect to that Emacs process."))
>       (kill-emacs 1))))

Yes.  With my patch, those messages will still show in exactly the same
cases.  No regression.

>> Currently the error message doesn't get shown at all, so I'm not clear
>> what could be regressing.
>
> We already established why the message doesn't show: because errors in
> server-start are not caught.  I thought we also agreed that catching
> the errors there and letting startup.el show the above error messages
> is something that should be done.

Yes.

> I think your patch will prevent us from doing that because the code
> which you suggest to patch with DAEMON_RUNNING will kill Emacs before
> startup.el gets the chance to show the error message, because
> daemon-initialized has not yet been run by that time.

Nope, it won't prevent us from doing that.  My patch affects only
*uncaught* errors, and improves behavior when errors are uncaught.  The
separate change we will make to the code you posted is to *catch more
errors*.  A change to the behavior for uncaught errors doesn't interfere
with us deciding to catch more errors.  Defining an explicit error
handler, of course, completely overrides the default handle for uncaught
errors.  So they're totally separate patches.