bug#33968: errors in shepherd service constructors are not logged and lead to misleading status

[Florian Dold <florian.dold@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

Hi Guix,

when defining a service type that extends shepherd-root-service-type and
the 'start' function of the shepherd-service definition contains an
error, the error is silently ignored.  No log output is generated at all.

For example (full system definition is attached):

(define (errtest-shepherd-service c)
  (list
    (shepherd-service
      (provision '(errtest))
      (documentation "Errtest")
      (requirement '(file-systems))
      (modules `((shepherd support) (ice-9 match) ,@%default-modules))
      (start #~(lambda args
                 (local-output "errtest start")
                 this-is-an-unbound-variable
                 (local-output "errtest end")
                 #t)))))


The log message "errtest start" appears in /var/log/messages, as
expected.  The next line contains an error, and aborts execution of the
start function.

The error only becomes apparent when manually doing a "herd restart
errtest", which shows an error message (but without any error location
or stack trace).  But the error (regarding the unbound variable) is not
logged, and there is no indication in the log that the service couldn't
be started in any log.

Furthermore the "herd status" of a service that encountered an error in
the start function is very misleading:

root@errtest ~# herd status errtest
Status of errtest:
  It is stopped.
  It is enabled.
  Provides (errtest).
  Requires (file-systems).
  Conflicts with ().
  Will be respawned.


It shows "Will be respawned", which is wrong.

I'd be happy to work on a patch, but it seems like there is some design
discussion necessary, in particular how the "Will be respawned" should
be handled.  Services have a "respawn?" flag, but of course respawning
can only work if the start function executed successfully (and only the
service process itself failed) in the first place.

I generally feel like the state machine for services needs some work.
In particular, it would be useful to distinguish between "failed" and
"completed" services instead of conflating both states into "stopped".
Or maybe have some more general mechanism for storing state about the
service, instead of just the slot that usually contains the PID?

- Florian

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: config-error-reporting.scm --]
[-- Type: text/x-scheme; name="config-error-reporting.scm", Size: 1253 bytes --]

(use-modules (gnu))
(use-service-modules networking ssh shepherd)

(define (errtest-shepherd-service c)
  (list
    (shepherd-service
      (provision '(errtest))
      (documentation "Errtest")
      (requirement '(file-systems))
      (modules `((shepherd support) (ice-9 match) ,@%default-modules))
      (start #~(lambda args
		 (local-output "errtest start")
		 this-is-an-unbound-variable
		 (local-output "errtest end")
		 #t)))))

(define errtest-service-type
  (service-type
   (name 'errtest)
   (extensions
    (list (service-extension shepherd-root-service-type errtest-shepherd-service)))
   (default-value #t)))

(operating-system
  (host-name "errtest")
  (timezone "Europe/Berlin")
  (locale "en_US.utf8")
  (kernel-arguments (list "console=ttyS0" "console=tty0"))
  (bootloader (bootloader-configuration
                (bootloader grub-bootloader)
                (target "/dev/sdX")))
  (file-systems (cons (file-system
                        (device (file-system-label "my-root"))
                        (mount-point "/")
                        (type "ext4"))
                      %base-file-systems))

  (services (cons* (service errtest-service-type)
                   %base-services)))