Skip to content
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

Ring middleware is not working with aleph adapter #350

Closed
g7s opened this issue Oct 15, 2019 · 3 comments
Closed

Ring middleware is not working with aleph adapter #350

g7s opened this issue Oct 15, 2019 · 3 comments

Comments

@g7s
Copy link

g7s commented Oct 15, 2019

Currently the aleph adapter returns a manifold Deferred response and ring middleware (such as ring.middleware.session) cannot work with such a value. Changing this deftype to:

(deftype AlephAsyncNetworkChannelAdapter []
  i/IServerChanAdapter
  (ring-req->server-ch-resp [sch-adapter ring-req callbacks-map]
    (let [{:keys [on-open on-close on-msg _on-error]} callbacks-map
          ws? (websocket-req? ring-req)]
      (if-let [s (and ws? (try @(aleph/websocket-connection ring-req)
                               (catch Exception e
                                 nil)))]
        (do
          (when on-msg   (s/consume     (fn [msg] (on-msg   s ws? msg)) s))
          (when on-close (s/on-closed s (fn []    (on-close s ws? nil))))
          (when on-open  (do                     (on-open  s ws?)))
          {:body s})
        (let [s (s/stream)]             ; sch
          (when on-close (s/on-closed s (fn [] (on-close s ws? nil))))
          (when on-open  (do                  (on-open  s ws?)))
          {:body s})))))

seems to work, but someone more qualified should take a better look

@ptaoussanis ptaoussanis self-assigned this Feb 23, 2023
@ptaoussanis ptaoussanis added this to the v1.18 milestone Feb 23, 2023
@ptaoussanis
Copy link
Member

@g7s Hi Gerasimos, thanks for pinging about this.
Unfortunately I'm not familiar with the Aleph API or possible tradeoffs with this change.

Since it's been a few years and no one else has given any input, I'll just incorporate your suggestion in the next pre-release and if anyone has any issues then we can revert.

@ptaoussanis
Copy link
Member

Closing, this change will be incorporated in forthcoming v1.18 release.

@g7s
Copy link
Author

g7s commented Feb 24, 2023

Thank you Peter 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants