Sync requests do not ignore `:else`
The plz docstring says "ELSE is an optional callback function... For synchronous requests, this argument is ignored."
It appears that sync requests do not ignore :else:
(plz 'get "https://fake.fake"
:else #'ignore)
returns nil instead of signalling an error.
Thanks. I should add a test for this.
Looking at this again, I'm not sure what the best API is. Would it be okay for synchronous requests to accept the ELSE argument for a function to call if the request fails? The choices look something like this:
(unless (ignore-errors
(plz 'get "https://fake.fake"))
...)
(or (ignore-errors
(plz 'get "https://fake.fake"))
...)
(unless (plz 'get "https://fake.fake"
:else #'ignore)
...)
(or (plz 'get "https://fake.fake"
:else #'ignore)
...)
And I'm not sure which is best. Maybe it would be okay to let users pass an ELSE, or maybe we should disallow it for sync requests. What do you think? (Probably we already discussed this and I forgot what we decided.)
FWIW, there is at least some code in the wild that uses :else #'ignore with sync requests, and would break if changed, e.g. https://github.com/jinnovation/kele.el/blob/15e841fb7bbc08545534e466ce831d6e80fd8901/kele.el#L459
@jinnovation Do you have any thoughts about this?
@Fuco1 I see that org-node-graph also uses plz (although I don't know if its code would be affected by changing this). What do you think?
@akirak I'd like to know what you think, too, if you would.
For synchronous requests I would most prefer it to throw an error I can handle with condition-case or ignore. I like that I can just let-bind the result and not worry about it, it makes the code very clean. In case of error, most likely I can't continue anyway so an error handler seems like a good place to jump to.
If an :else is there, plz can somehow handle the error internally and use that form/function as the error handler? But maybe I would deprecate such use.
I prefer the documented behavior over the current behavior, but I don't feel strongly about it.
I'd like to know what you think, too, if you would.
As you might know, the uniform API to chain asynchronous operations is promises.
async/await is a syntactic pattern that incorporates promises into synchronous computation. I never recommend doing things like below, because it doesn't handle errors and can lead to an unexpected behavior:
(unless (ignore-errors
(plz 'get "https://fake.fake"))
...)
(or (ignore-errors
(plz 'get "https://fake.fake"))
...)
I personally find promise.el the easiest to use among several promise libraries. Its promise-chain form is what one should use here.
I've only used plz.el for simple use cases, so I didn't pay much attention to the :else argument. My usage has been only synchronous and has had no error handling (which would be bad in mission critical software but my Emacs packages are nothing like that). For complex things, it would be nice if there were support for ease of integration with promise.el.
@akirak FYI, you might be interested in this (though it uses aio rather than promise):
https://github.com/alphapapa/plz.el/blob/wip/aio/aio-plz.el https://github.com/alphapapa/plz.el/blob/wip/aio/tests/test-aio-plz.el
Since this is not a new bug or issue, and I'm still not sure how I want to handle it, I'm deferring it to v0.8 so that v0.7 can be tagged now.