Improve error handling if process is killed

[leotaku]

Created as a response to https://github.com/alphapapa/plz.el/pull/68#issuecomment-2387082252.

If I understand the plz error handling code correctly, it currently only checks and reports if the curl process was killed if the returned error code is above 90. This intent is clarified in a code comment.

However, on my machine, curl does not report an error code above 90 if the process is killed. In most cases, I see the error code 9, which normally means "FTP access denied", but seems to be mostly random here.

This can be confirmed with the following code snippet.

(let ((p (start-process "test" nil "curl" "https://example.com")))
  (kill-process p)
  (run-with-timer 1 nil (lambda (p) (message "%s" (process-exit-status p))) p))
;; => prints "9" after 1 second

Killing the returned process seems to be the default way to "cancel" a request made using plz, so I think the current behavior is confusing. Also, there is clearly already code there to separately handle killed or interrupted processes, it just is never reached in my experience.

Best wishes

[alphapapa]

Thanks for reporting this. I've begun seeing similar behavior myself, i.e. mysterious code 9, "FTP access denied" messages instead of what's expected. It almost suggests that something, somewhere is omitting the 0 from 90, although it should be an integer to begin with.

I'm not sure exactly when it started happening. Probably we should test with some earlier versions of plz, and if it still happens, maybe on an earlier version of Emacs.

My time for Emacs stuff is limited at the moment, so any help would be welcome.

[leotaku]

The "new" error handling code seems to have been introduced in https://github.com/alphapapa/plz.el/commit/46d0c54525a8d598960162d2717aecbab3d85b03, so I guess newest version it might make sense to test would be that one. I'm not sure what testing with older Emacsen would achieve? Maybe testing with older curl versions might make sense.

However, at least for me, the issue seems very obvious:

1. The error handling code assumes that curl will return an error code >90 when it is killed
2. curl does not always return an error code >90 when killed

So I am unsure what is left to test? Is there a specific reason the current code expects curl to return an error code >90 when killed? I wasn't able to find any documentation for this behavior in curl.

[alphapapa]

I think you're right that there is a bug here that incorrectly depends on the exit code in circumstances where the code is not relevant, e.g. when the process is killed. I can't seem to find any details about what the exit code should be when a process is killed. Someone sufficiently motivated could go spelunking in process.c and maybe find an answer, but that might not be important, anyway.

I wish there were a canonical example of how to handle this in a process sentinel, but I don't know of one. In the Elisp manual, section "40.10 Sentinels: Detecting Process Status Changes", it even admits that the string argument passed to the sentinel may have values other than those listed there, so it's hard to know exactly what to look for in some cases. In fact, "killed\n" (which is what I've observed it to be when a process is killed) is not even listed there, so it seems necessary to do some experimentation to handle this case (and until recently, it seemed to handle this case correctly).

Anyway, I do agree that this should be fixed. I also hope that, when applying fixes, we can add test cases to ensure that the fixes work as intended. (And if we were to be really diligent, we might even suggest relevant improvements to the Elisp manual.)

So that would be my first to-do item, if I were working on this (which I will eventually, if no one else does, but again, my time is quite limited at the moment): to write tests for this problem (i.e. killing and interrupting the curl process, and handling the error as expected).

Thanks for your help with this.