Add an option to set the process filter

[r0man]

The process filter is an optional function to be used as the process filter for the curl process. It can be used to handle HTTP responses in a streaming way. The function must accept 2 arguments, the process object running curl, and a string which is output received from the process. The default process filter inserts the output of the process into the process buffer. The provided function should at least insert output up to the HTTP body into the process buffer.

[r0man]

I think I found the issue I was talking about. Let me add another test for this and a fix.

[r0man]

The plz-get-json-slow-process-filter demonstrates the issue that raises this error:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
  <=(200 nil 299)
  (let* ((status val)) (<= 200 status 299))
  (if (let* ((status val)) (<= 200 status 299)) (let ((status val)) (ignore status) (ignore status) (funcall (process-get process :plz-then))) (let nil (let ((err (make-plz-error :response (plz--response)))) (let* ((val (process-get process :plz-else))) (cond ((null val) (let nil (process-put process :plz-result err))) ((functionp val) (let (...) (funcall fn err))) (t (let (...) (error "No clause matching `%S'" x80))))))))

In the LLM library user code gets called and the process finishes while the process filter is still active. So I think we need a way to tell plz--sentinel to only call plz--respond once the filter is actually done.

The reason why this only happens sometimes when using the LLM library is probably, because in rare cases, the process finished while my process filter was still active. And plz--respond was called in a moment where my process filter had narrowed the buffer. The call to widen worked in that case.

In the test above there the process filter does not narrow. But the process finishes and the process filter hasn't even finished writing to the buffer because of the sit-for.

Does that make sense? Any ideas how to solve this? Maybe using accept-process-output or a :watermark on the process that is initialized to 0, incremented by 1 when the process filter enters, and decremented when it exits.

[r0man]

@alphapapa Ok, I think I got it working. I deadlocked myself by trying to use sleep-for or sit-in in plz and the tests at the same time. I learned about run-at-time which seems to be working. I will try this tomorrow with plz-media-type and the llm library.

[r0man]

I tested this now with Ellama and the llm library and haven't noticed any issues so far.

[alphapapa]

In the LLM library user code gets called and the process finishes while the process filter is still active. So I think we need a way to tell plz--sentinel to only call plz--respond once the filter is actually done.

That could indicate a design flaw in the filter function. AFAIK the filter should be as simple as possible. If any work needs to be done, being dispatched from the filter, it should probably be done outside of the filter's call stack, so that the filter can return quickly.

The reason why this only happens sometimes when using the LLM library is probably, because in rare cases, the process finished while my process filter was still active. And plz--respond was called in a moment where my process filter had narrowed the buffer. The call to widen worked in that case.

AFAIK there is never any guarantee about whether a process will still be running when a filter or sentinel is called. So if a filter or sentinel changes state that could affect other computations (like narrowing or widening a buffer), it should generally restore that state before returning (and should use unwind-protect to ensure that it happens) .

In the test above there the process filter does not narrow. But the process finishes and the process filter hasn't even finished writing to the buffer because of the sit-for.

Using sit-for in the tests is only done for asynchronous processes, and only because the tests themselves are synchronous. Outside of tests, it should generally be avoided as much as possible, and code should not be written with any expectations about such calls.

Does that make sense? Any ideas how to solve this? Maybe using accept-process-output or a :watermark on the process that is initialized to 0, incremented by 1 when the process filter enters, and decremented when it exits.

AFAIK trying to control how the sentinel behaves from within the filter, or vice versa, is generally a Bad Idea, likely to lead to weird problems like the ones you've described. I've already dealt with trying to dispatch work from within the sentinel causing problems, which is why an immediately run timer is now used instead. So I would generally decline to add something like the "filter mark" property as a kind of mutex between them. I think it would be a tangled web to weave.

Speaking generally, you probably need to design the filter carefully, ensuring that it does the least work possible, and that it restores any global state that could affect other computation. It should have no expectations about when it will be called relative to other functions, what any global state will be, etc.

IME this is a "hairy" area of Emacs. process.c is not easy to follow or work on. From what I know, work on it has tended to follow a pattern of people fixing problems that they encounter and then moving on; so it gradually accumulates changes, but it doesn't get high-level refactoring, and any changes come with the risk of breaking previous ones.

So at the Lisp level, some degree of trial-and-error is sometimes needed, as well as very careful study of the Elisp manual, and sometimes reference to existing, related code. It's not a fast process: getting plz to the point of reliability and maturity took a few years of experimentation, and eventually the final fix happened because another developer happened to share some code he had written to solve a similar process-related problem.

So while I'm happy for you to build on plz and use it in your projects, merging changes back into it is necessarily a very conservative matter.

For setting the process filter, I'm happy to simply add an option to pass through the argument to make-process. If you need anything more than that, it should probably be developed in your own project; then, after extensive testing and use "in anger," if it seems more generally useful, it could be considered for merging into plz.

Does this make sense? Thanks.

[r0man]

Hi, yes, makes sense. I agree this process filter stuff is tricky. Even my "solution" took me a while. I will try to see how I get it fixed in plz-media-type.

I updated the PR to now only include setting the filter options. And my paperwork with the FSF is also complete since yesterday, I received back the signed documents from the FSF.

[alphapapa]

Hi, yes, makes sense. I agree this process filter stuff is tricky. Even my "solution" took me a while. I will try to see how I get it fixed in plz-media-type.

Sounds good.

I updated the PR to now only include setting the filter options. And my paperwork with the FSF is also complete since yesterday, I received back the signed documents from the FSF.

Ok, would you ask the FSF secretary to send confirmation to me, please? I've been told by the Emacs maintainers that I should confirm these signings from the FSF rather than taking the word of the contributor. :)

Thanks.

[r0man]

Ok, I will ask him.

[r0man]

@alphapapa Did Craig from the FSF reach out to you? I contacted him, and he mentioned I should be able to contribute since the 1st of February (I think he meant 1st of April though), and that he is going to contact you. He also mentioned that there might be a misunderstanding in the process.

[r0man]

@alphapapa Can we please somehow move forward with this? An alternative way to get the green light would be to ask on emacs-devel? Eli got a copy of my assignment already 3 weeks ago and the ELPA maintainers as well.

[alphapapa]

Hi Roman,

I received your emails to Craig that you cc'ed me on, but I didn't receive an email from Craig about you. I have received confirmation emails from Craig about another contributor to another project.

But I see the PDF of your confirmation that you attached to the email that you cc'ed me on, so I think that should serve as confirmation.

[alphapapa]

@r0man Merged. Thanks for your work on this, and for your patience.

[r0man]

Hi @alphapapa, alright, thanks for merging this. Would you be able to cut a release with this in the near future, so we can depend on that version?

[alphapapa]

@r0man Ok, I just released v0.8 with this commit.

[r0man]

Thanks @alphapapa, much appreciated!