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

Returning a java.net.URL or java.net.URI at the REPL causes CIDER to try to connect #2825

Closed
dgr opened this issue Mar 26, 2020 · 12 comments
Closed
Labels
bug cider-nrepl The issue is related to cider-nrepl stale

Comments

@dgr
Copy link
Contributor

dgr commented Mar 26, 2020

I'm using the latest CIDER snapshot as of March 25, 2020, and when I create a URI or URL at the CIDER REPL, it prints the representation of the returned object (e.g. #object[java.net.URI ...]) but then it tries to connect to the URI/URL. If the URI/URL can be connected to, this will return, for instance, an HTML document. If not, it will throw an error.

I would not expect this behavior since returning a URI or URL from a function would be a very normal thing to do in the case of REPL-driven development. If I wanted to connect to the URI/URL, I would expect to use cli-http or something like that.

EXAMPLE:

user> (java.net.URI. "mailto:foo@bar.com")
#object[java.net.URI 0x36966ca2 "mailto:foo@bar.com"]ERROR: Unhandled REPL handler exception processing message {:op slurp, :url mailto:foo@bar.com, :session 69c4d8e1-7bb4-45ad-8075-d21995fd50ab, :id 1579}
java.net.UnknownServiceException: protocol doesn't support input
	at java.base/java.net.URLConnection.getInputStream(URLConnection.java:840)
	at cider.nrepl.middleware.slurp$slurp_url_to_content_PLUS_body.invokeStatic(slurp.clj:100)
	at cider.nrepl.middleware.slurp$slurp_url_to_content_PLUS_body.invoke(slurp.clj:82)
	at cider.nrepl.middleware.slurp$handle_slurp.invokeStatic(slurp.clj:117)
	at cider.nrepl.middleware.slurp$handle_slurp.invoke(slurp.clj:109)
	at clojure.lang.Var.invoke(Var.java:388)
	at cider.nrepl$wrap_slurp$fn__8316.invoke(nrepl.clj:95)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_xref$fn__8496.invoke(nrepl.clj:473)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_spec$fn__8440.invoke(nrepl.clj:381)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_resource$fn__8432.invoke(nrepl.clj:369)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_macroexpand$fn__8392.invoke(nrepl.clj:250)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_info$fn__8374.invoke(nrepl.clj:181)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_clojuredocs$fn__8504.invoke(nrepl.clj:488)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_classpath$fn__8332.invoke(nrepl.clj:111)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_version$fn__8488.invoke(nrepl.clj:463)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.interruptible_eval$interruptible_eval$fn__7628.invoke(interruptible_eval.clj:155)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_out$fn__8408.invoke(nrepl.clj:297)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_debug$fn__8350.invoke(nrepl.clj:137)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cruxplay.core$eval8587$fn__8588$fn__8590.invoke(form-init11138167219954525445.clj:1)
	at nrepl.middleware.session$add_stdin$fn__7777.invoke(session.clj:351)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_inspect$fn__8384.invoke(nrepl.clj:200)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_enlighten$fn__8358.invoke(nrepl.clj:163)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.load_file$wrap_load_file$fn__7666.invoke(load_file.clj:81)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_content_type$fn__8308.invoke(nrepl.clj:83)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.caught$wrap_caught$fn__7569.invoke(caught.clj:97)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.print$wrap_print$fn__7536.invoke(print.clj:234)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_undef$fn__8480.invoke(nrepl.clj:455)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_ns$fn__8400.invoke(nrepl.clj:263)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_trace$fn__8464.invoke(nrepl.clj:426)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_tracker$fn__8472.invoke(nrepl.clj:444)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.sideloader$wrap_sideloader$fn__7825.invoke(sideloader.clj:102)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at cider.nrepl$wrap_complete$fn__8340.invoke(nrepl.clj:117)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.middleware.session$session$fn__7762.invoke(session.clj:297)
	at nrepl.middleware$wrap_conj_descriptor$fn__7309.invoke(middleware.clj:16)
	at nrepl.server$handle_STAR_.invokeStatic(server.clj:19)
	at nrepl.server$handle_STAR_.invoke(server.clj:16)
	at nrepl.server$handle$fn__7840.invoke(server.clj:36)
	at clojure.core$binding_conveyor_fn$fn__5754.invoke(core.clj:2030)
	at clojure.lang.AFn.call(AFn.java:18)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)
@dpsutton
Copy link
Contributor

@dgr you're perfectly right to be confused. This is part of a feature to display images in the repl buffer. However it has knock on effects of connecting to every URL or URI that's returned to the repl which is quite dangerous in my opinion. Try the following emacs lisp to disable it:

(setq cider-repl-use-content-types nil)

@dgr
Copy link
Contributor Author

dgr commented Mar 29, 2020

@dgr you're perfectly right to be confused. This is part of a feature to display images in the repl buffer. However it has knock on effects of connecting to every URL or URI that's returned to the repl which is quite dangerous in my opinion. Try the following emacs lisp to disable it:

(setq cider-repl-use-content-types nil)

Thanks for the reply. I'll disable that. Yea, if you were developing an API, an unexpected connection to your running system could delete something in your database, all because you were fiddling around with the code to create appropriate URIs. Displaying images seems like a generally bad trade. Or maybe it's just bad to set the defaults that way; maybe reverse them, so that you have to purposefully enable this functionality.

@bbatsov
Copy link
Member

bbatsov commented Mar 29, 2020

Yeah, I'll likely disable this in future CIDER release. Still, I don't think the feature is that problematic. @Cartmanishere recently fixed a couple of similar problems there, I hope he'll has the time to look into this one as well.

@bbatsov bbatsov added bug cider-nrepl The issue is related to cider-nrepl labels Mar 29, 2020
@bbatsov
Copy link
Member

bbatsov commented Apr 13, 2020

FYI - I've disabled content-type by default.

@stale
Copy link

stale bot commented Jul 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale label Jul 12, 2020
@bbatsov bbatsov closed this as completed Jul 12, 2020
@harold
Copy link
Contributor

harold commented Oct 16, 2020

Upon quick inspection, these seem to be the offending lines:
https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/content_type.clj#L91-L99

The image handling appears to be separate, which is cool.

Getting rid of those lines would break things like (clojure.java.io/file "image.png") from displaying the image in the repl. But one could still do (javax.imageio.ImageIO/read (clojure.java.io/file "image.png")) 😄

I'd be up for playing around with this, but I don't know how to run CIDER with changed nrepl middleware. Is there a hacking guide somewhere?

As ever, thanks to all for time and consideration and efforts on these projects we love and use and love to use.


PS. I landed here from this part of the docs which had a link: https://docs.cider.mx/cider/repl/configuration.html#displaying-images-in-the-repl

@bbatsov
Copy link
Member

bbatsov commented Oct 16, 2020

@harold Check out https://docs.cider.mx/cider/0.26/contributing/hacking.html

@harold
Copy link
Contributor

harold commented Oct 16, 2020

Outstanding! --- Thank you.

@harold
Copy link
Contributor

harold commented Oct 16, 2020

Looked at this a bit more this afternoon.

It's all very clever, and I learned a lot and has some laughs, so thanks as ever for that.

The URI/URL behavior is actually kinda cute because it allows an expression like (java.net.URI. "https://techascent.com/img/laptop-cloud.png") to actually print an image in the repl. 🥜

image

But, of course, it does that by fetching the contents - which we've all found surprising in other contexts (http APIs and pages for instance).

What I aimed to accomplish was to eliminate the surprising bit, but preserve the fun of printing images.

I've got this in my config now:

(setq cider-repl-use-content-types t)
(defun rewrite-content-type-list ()
  (setq cider-repl-content-type-handler-alist
	(seq-filter (lambda (item) (not (string= (car item)
						 "message/external-body")))
		    cider-repl-content-type-handler-alist)))
(add-hook 'cider-repl-mode-hook #'rewrite-content-type-list)

It works by disabling the message/external-body (https://github.com/clojure-emacs/cider/blob/master/cider-repl.el#L925) content-type handling at the cider-repl.el level, which was kicking off the slurp (https://github.com/clojure-emacs/cider/blob/master/cider-repl.el#L919), and ultimately leading to the fetching and the confusion.

Things are pretty good now:
image

There are lots of other potential fixes (and improvements to the related 'content-type' and 'slurp' nrepl middleware), but getting a nice middleground behavior by just configuring cider from the outside feels ok for now.

🙇‍♂️ - respect to all involved, as ever.

[@cnuernber - mildly relevant to your interests]

@bbatsov
Copy link
Member

bbatsov commented Oct 17, 2020

Thanks for looking into this! I guess we can extend the docs a bit to cover your findings.

@bbatsov
Copy link
Member

bbatsov commented Oct 17, 2020

Btw, I don't think you actually need the hook function - you could have just modified cider-repl-content-type-handler-alist directly IMO.

@harold
Copy link
Contributor

harold commented Oct 17, 2020

I guess we can extend the docs a bit to cover your findings.

Sure. Or this issue on its own may help someone down the line.

I don't think you actually need the hook function

I thought so too, but I got a message at startup about cider-repl-content-type-handler-alist being empty, so that's when I switched to the hook function. I lack the expertise to say precisely what happened. Maybe that defvar isn't ready before a repl is started?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cider-nrepl The issue is related to cider-nrepl stale
Projects
None yet
Development

No branches or pull requests

4 participants