-
-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core/abort fails on client/request #46
Comments
Patch welcome! |
I think something like this might work; haven't checked that it doesn't break things horribly yet: https://github.com/rjohnsondev/cljs-http/compare/fix/abort-request?expand=1 |
That is interesting implementation. It could work. One note: it would be best to do get, assoc and dissoc in one "transaction": e.g. (defn abortable-async-map [f c]
(let [new-c (async/map f [c])]
(swap! core/pending-requests (fn [p] (-> p (dissoc c) (assoc new-c (get p c)))) |
FYI I did try this however it causes a bunch of the tests to fail for some reason, will dig into it more when I have time |
It took me a while to realise that the abort function here https://github.com/r0man/cljs-http/blob/master/src/cljs_http/core.cljs#L9 is not compatible with channels generated by
client/request
and friends (due to the response wrappers? https://github.com/r0man/cljs-http/blob/master/src/cljs_http/client.cljs#L97; since 9fe48a7 ?)I saw the commits by @Deraen with the additional cancel channel which can be used to cancel a request and managed to get that method working; however I would have preferred not to have created an extra channel... (also should this be called abort rather than cancel?)
Anyway, can I recommend a change in the documentation to advise unsuspecting ppl that
core/abort
shouldn't be used againstclient/request
; and request that it's either made compatible, or an additional abort function that works withclient/request
be added toclient.clj
?The text was updated successfully, but these errors were encountered: