emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
226 stars 17 forks source link

Blocking GET requests error out when run under eldev #79

Closed edran closed 1 year ago

edran commented 1 year ago

Hi there,

I've noticed any GET request made with request seem to fail when they are run synchronously.

To reproduce the issue, I took one of the examples in their readme, chucked it into a file:

(require 'request)
(require 'buttercup)

(defun httpbin-get-test (sync)
  (interactive)
  (request "http://httpbin.org/get"
        :params '(("key" . "value") ("key2" . "value2"))
        :sync sync
        :parser 'json-read
        :success (cl-function
                  (lambda (&key data &allow-other-keys)
                    (message "I sent: %S" (assoc-default 'args data))))))

(describe "`httpbin-get-test' will"
  (describe "with :sync f"
    (it "successfully runs"
      (expect (type-of (httpbin-get-test 'nil)) :to-be 'request-response)))
  (describe "with :sync t"
    (it "crashes :-( (but shouldn't?)"
      (expect (type-of (httpbin-get-test t)) :to-be 'request-response))))

And run eldev test:

Running 2 specs.

`httpbin-get-test' will
  with :sync f
    successfully runs (4.05ms)
  with :sync t
    crashes :-( (but shouldn't?)  FAILED (0.33ms)

========================================
`httpbin-get-test' will with :sync t crashes :-( (but shouldn't?)

Traceback (most recent call last):
  (type-of (httpbin-get-test t))
  httpbin-get-test(t)
  request("http://httpbin.org/get" :params (("key" . "value") ("key2" . "value2")) :sync t :parser json-read :success (closure ((sync . t) t) (&rest --cl-rest--) "\n\n(fn &key DATA &allow-other-keys)" (let* ((data (car (cdr (plist-member --cl-rest-- ':data))))) (message "I sent: %S" (assoc-default 'args data)))))
  apply(request--curl-sync "http://httpbin.org/get?key=value&key2=value2" (:params (("key" . "value") ("key2" . "value2")) :sync t :parser json-read :success (closure ((sync . t) t) (&rest --cl-rest--) "\n\n(fn &key DATA &allow-other-keys)" (let* ((data (car (cdr (plist-member --cl-rest-- ':data))))) (message "I sent: %S" (assoc-default 'args data)))) :error #[128 "\302\300\303\301\"\"\207" [request-default-error-callback ("http://httpbin.org/get") apply append] 6 "\n\n(fn &rest ARGS2)"] :url "http://httpbin.org/get?key=value&key2=value2" :response #s(request-response nil nil nil nil nil "http://httpbin.org/get?key=value&key2=value2" nil #1 #<buffer  *request curl*-69972> nil nil curl nil) :encoding utf-8))
  request--curl-sync("http://httpbin.org/get?key=value&key2=value2" :params (("key" . "value") ("key2" . "value2")) :sync t :parser json-read :success (closure ((sync . t) t) (&rest --cl-rest--) "\n\n(fn &key DATA &allow-other-keys)" (let* ((data (car (cdr (plist-member --cl-rest-- ':data))))) (message "I sent: %S" (assoc-default 'args data)))) :error #[128 "\302\300\303\301\"\"\207" [request-default-error-callback ("http://httpbin.org/get") apply append] 6 "\n\n(fn &rest ARGS2)"] :url "http://httpbin.org/get?key=value&key2=value2" :response #s(request-response nil nil nil nil nil "http://httpbin.org/get?key=value&key2=value2" nil (:params (("key" . "value") ("key2" . "value2")) :sync t :parser json-read :success (closure ((sync . t) t) (&rest --cl-rest--) "\n\n(fn &key DATA &allow-other-keys)" (let* ((data (car (cdr (plist-member --cl-rest-- ':data))))) (message "I sent: %S" (assoc-default 'args data)))) :error #[128 "\302\300\303\301\"\"\207" [request-default-error-callback ("http://httpbin.org/get") apply append] 6 "\n\n(fn &rest ARGS2)"] :url "http://httpbin.org/get?key=value&key2=value2" :response #1 :encoding utf-8) #<buffer  *request curl*-69972> nil nil curl nil) :encoding utf-8)
  request-auto-revert-notify-rm-watch()
error: (void-variable auto-revert-notify-watch-descriptor-hash-list)

Ran 2 specs, 1 failed, in 6.22ms.
1 Buttercup test failed

I can also confirm that buttercup runs both correctly if launched interactively within Emacs.


System info:

$ eldev dependencies
request 0.3.2

$ emacs --version
GNU Emacs 28.2
Copyright (C) 2022 Free Software Foundation, Inc.
GNU Emacs comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of GNU Emacs
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.

$ uname -a
Darwin eschatologist 21.6.0 Darwin Kernel Version 21.6.0: Sun Nov  6 23:31:13 PST 2022; root:xnu-8020.240.14~1/RELEASE_ARM64_T6000 arm64
doublep commented 1 year ago

I cannot reproduce this and also this doesn't make sense to me. The only case this variable is referred to in request's source code is this:

             (when (boundp 'auto-revert-notify-watch-descriptor-hash-list)
               auto-revert-notify-watch-descriptor-hash-list))))

I don't see how this could ever result in void-variable error. Maybe you have an outdated package?

doublep commented 1 year ago

Sorry, wrote too soon.

$ eldev dependencies
request 0.3.2

When I check out that tag, then yes, I get the same error.

It's not a bug in Eldev, it's a bug in request. Some projects create a few stable releases (e.g. 0.3.2) only to stop and never release again. This bug was fixed over two years ago in this commit: 0183da8 include 'make compile' in 'make test'.

The proper way is to force request to make a stable version. A workaround is to use an unstable package.

edran commented 1 year ago

Understood. Thank you for the pointer, I'll open an issue in request :-)