Missing sentinel events

[Lars Ingebrigtsen]

While looking at all the failure modes, we seem to be lacking a few
during async connection.

The manual says that sentinels get codes like:

-----
The string describing the event looks like one of the following:

    "finished\n".
    "deleted\n".
    "exited abnormally with code exitcode (core dumped)\n". The “core
    dumped” part is optional, and only appears if the process dumped
    core.
    "failed with code fail-code\n".
    [many more removed here]
---

But this is the code from process.c:

  else if (p->outfd < 0)
    {
      /* The counterparty may have closed the connection (especially
         if the NSM prompt above take a long time), so recheck the file
         descriptor here. */
      pset_status (p, Qfailed);
      deactivate_process (proc);
    }
  else if ((fd_callback_info[p->outfd].flags & NON_BLOCKING_CONNECT_FD) == 0)
    {
      /* If we cleared the connection wait mask before we did the TLS
         setup, then we have to say that the process is finally "open"
         here. */
      pset_status (p, Qrun);
      /* Execute the sentinel here.  If we had relied on status_notify
         to do it later, it will read input from the process before
         calling the sentinel.  */
      exec_sentinel (proc, build_string ("open\n"));
    }

I suspect that I just forgot to put those calls to exec_sentinel into
the "fail" bits here and other places in the code when I rewrote these
parts last February, like this:

  /* The DNS lookup failed. */
  else if (connecting_status (p->status))
    {
      deactivate_process (proc);
      pset_status (p, (list2
                       (Qfailed,
                        concat3 (build_string ("Name lookup of "),
                                 build_string (p->dns_request->ar_name),
                                 build_string (" failed")))));
    }

Doesn't it seem like these should be calling the sentinel, too?  I think
so, but there's a slight chance that adding more sentinel calls will
break some user-level code that's not expecting to see any new
messages...

And the manual says "looks like one of the following", and people may
have interpreted that as an exhaustive list of event strings.

Any thoughts?  It's incomplete as it is now, so we should definitely add
more sentinel calls, but should they all look like "failed with code
fail-code\n" or should they be... more meaningful?

I think perhaps I should just add "more meaningful" sentinel messages,
and if it turns out to be a problem in real life, we can change them all
to "failed with code" messages...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no