Fix the synchronous mode in sparql-execute-query

olejorgenb commented 8 years ago

This code-path was very buggy

putting the result in a temp buffer instead of the sparql-results-buffer
not using the a fallback value for url

I know it isn't even possible to use this flag without changing the code but I needed to run things synchronously due to a (AFAICS) bug in url-retrieve:

(url-mime-accept-string (or format (sparql-get-format))) Have no effect when using url-retrieve asynchronously...

ljos commented 8 years ago

Thank you very much. Sorry for leaving it broken. I haven't been using this mode much myself in a while. I see, perhaps, some other issues in there. I will have to look into that soon...

olejorgenb commented 8 years ago

np, it was just a nice excuse to play a bit with elisp.

Thanks for creating the mode in the first place :)

ljos commented 8 years ago

However, I am not sure it is true that the accept header is never passed along when using async. I see that it sometimes is not, but then other times it is.

ljos commented 8 years ago

I just realized, there is a reason the result was put in a temp buffer. The synchronous mode was meant for use in ob-sparql and was supposed to just return the results instead of putting it into a separate buffer.

olejorgenb commented 8 years ago

Ah, that explains a bit (in that code path the url is passed explicit, etc.) Not good that I didn't check the call stack across files.

However, I am not sure it is true that the accept header is never passed along when using async. I see that it sometimes is not, but then other times it is.

I did dig into the emacs code and (with the disclaimer that I'm not a elisp veteran) it seems like the url-mime-accept-string variable usually was read by the async code too late.

client code calls url-retrieve with let binding of url-mime-accept-string
url-retrieve kicks of a async request, but does not create the request parameters just yet.
client code returns and the let binding is "expired" (correct me if I'm wrong here)
the async request is picked up and the request parameters are constructed from the now nil again url-mime-accept-string

This can be seen in url-http-async-sentinel and url-http and the result can be observed by setting url-debug to t.

Kinda weird that there is such a bug in a core (?) emacs library. Again, I could overlook something of course. An to be fair, the documentation of url-retrieve actually says:

The variables url-request-data',url-request-method' and `url-request-extra-headers' can be dynamically bound around the request; dynamic binding of other variables doesn't necessarily take effect.

If I pass the "Accept" header through the url-request-extra-headers it is reliable passed, but the final request ends up having two "Accept" headers, "/" and the one passed by us. The one passed in extra headers comes last, so I guess it'll override "/"?

This is only judging from the URL_DEBUG output. I haven't checked what arrives at the server properly.

If I do two rapid queries the latter have the correct header. Which make sense since it looks like the emacs code might reuse the connection or something, and in that case the request parameters are constructed earlier.

ljos commented 8 years ago

I reported it to the emacs bug tracker and it has already been fixed in the newest version: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22855

They say that the reason it works the second time is because emacs caches the connection which means it gets the binding in time the second run.

I will revert back to using just the system libraries when they release the new version.

olejorgenb commented 8 years ago

Great :)

The commit in emacs for the weirdly interested ones: http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=144bb0cf322b9756d29def3e27a42303e2edce43

ljos / sparql-mode

Fix the synchronous mode in sparql-execute-query #47