:arglists in lookup op response map cannot always be serialized into Bencode

[eerohele]

Steps to reproduce the problem

(def output (java.io.ByteArrayOutputStream.))
(write-bencode output {{:foo 1} :bar})
;; or
(write-bencode output {[:foo] :bar})

Throws NPE:

Execution error (NullPointerException) at nrepl.bencode/string>payload (bencode.clj:175).
null
{:via [{:type java.lang.NullPointerException,
        :at [nrepl.bencode$string_GT_payload invokeStatic "bencode.clj" 175]}],
 :trace [[nrepl.bencode$string_GT_payload invokeStatic "bencode.clj" 175]
         [nrepl.bencode$string_GT_payload invoke "bencode.clj" 173]
         [clojure.core$comp$fn__5807 invoke "core.clj" 2569]
         [clojure.core$juxt$fn__5822 invoke "core.clj" 2594]
         [clojure.core$map$fn__5866 invoke "core.clj" 2755]
         [clojure.lang.LazySeq sval "LazySeq.java" 42]
         [clojure.lang.LazySeq seq "LazySeq.java" 51]
         [clojure.lang.RT seq "RT.java" 535]
         [clojure.core$seq__5402 invokeStatic "core.clj" 137]
         [clojure.core.protocols$seq_reduce invokeStatic "protocols.clj" 24]
         [clojure.core.protocols$fn__8146 invokeStatic "protocols.clj" 75]
         [clojure.core.protocols$fn__8146 invoke "protocols.clj" 75]
         [clojure.core.protocols$fn__8088$G__8083__8101 invoke "protocols.clj" 13]
         [clojure.core$reduce invokeStatic "core.clj" 6828]
         [clojure.core$into invokeStatic "core.clj" 6895]
         [clojure.core$into invoke "core.clj" 6887]
         [nrepl.bencode$eval3461$fn__3462 invoke "bencode.clj" 405]
         [clojure.lang.MultiFn invoke "MultiFn.java" 234]
         [nrepl.bencode$eval3474 invokeStatic "form-init10775299742697767996.clj" 1]
         [nrepl.bencode$eval3474 invoke "form-init10775299742697767996.clj" 1]
         [clojure.lang.Compiler eval "Compiler.java" 7177]
         [clojure.lang.Compiler eval "Compiler.java" 7132]
         [clojure.core$eval invokeStatic "core.clj" 3214]
         [clojure.core$eval invoke "core.clj" 3210]
         [nrepl.middleware.interruptible_eval$evaluate$fn__953$fn__954
          invoke
          "interruptible_eval.clj"
          82]
         [clojure.lang.AFn applyToHelper "AFn.java" 152]
         [clojure.lang.AFn applyTo "AFn.java" 144]
         [clojure.core$apply invokeStatic "core.clj" 665]
         [clojure.core$with_bindings_STAR_ invokeStatic "core.clj" 1973]
         [clojure.core$with_bindings_STAR_ doInvoke "core.clj" 1973]
         [clojure.lang.RestFn invoke "RestFn.java" 425]
         [nrepl.middleware.interruptible_eval$evaluate$fn__953 invoke "interruptible_eval.clj" 82]
         [clojure.main$repl$read_eval_print__9086$fn__9089 invoke "main.clj" 437]
         [clojure.main$repl$read_eval_print__9086 invoke "main.clj" 437]
         [clojure.main$repl$fn__9095 invoke "main.clj" 458]
         [clojure.main$repl invokeStatic "main.clj" 458]
         [clojure.main$repl doInvoke "main.clj" 368]
         [clojure.lang.RestFn invoke "RestFn.java" 1523]
         [nrepl.middleware.interruptible_eval$evaluate invokeStatic "interruptible_eval.clj" 79]
         [nrepl.middleware.interruptible_eval$evaluate invoke "interruptible_eval.clj" 56]
         [nrepl.middleware.interruptible_eval$interruptible_eval$fn__984$fn__988
          invoke
          "interruptible_eval.clj"
          145]
         [clojure.lang.AFn run "AFn.java" 22]
         [nrepl.middleware.session$session_exec$main_loop__1051$fn__1055 invoke "session.clj" 202]
         [nrepl.middleware.session$session_exec$main_loop__1051 invoke "session.clj" 201]
         [clojure.lang.AFn run "AFn.java" 22]
         [java.lang.Thread run "Thread.java" 834]]}

Environment & Version information

Clojure version

1.10.1

Java version

11

Operating system

macOS

I can look into fixing the issue if I get the chance. Just wanted to file the issue so that I don't forget.

[SevereOverfl0w]

bencode cannot represent non-string keys in maps: https://en.wikipedia.org/wiki/Bencode

[eerohele]

Ah. Well, that's a bummer. The place where I noticed this is the lookup op, which fails if the :arglists key has a value that has a collection as a key.

I guess one option would be to drop the :arglists key and only retain :arglists-str? That would be a breaking change at this point, though.

[bbatsov]

It's an experimental feature, so it's not really a breaking change. That being said another approach might be to remove this key depending on the structure of the arglists or to convert it to string automatically when it can't be encoded.

[eerohele]

It's an experimental feature, so it's not really a breaking change.

All right. FWIW, I only use :arglists-str myself.

That being said another approach might be to remove this key depending on the structure of the arglists

An arglist of a function that makes heavy use of destructuring might be quite complex. It would certainly be doable, but it might also incur a performance cost. Probably not a large one, but still. Not sure the gains are large enough to warrant the additional complexity.

or to convert it to string automatically when it can't be encoded.

As a consumer, I would prefer it if the type of the return value of the op were predictable.

My vote would be on dropping :arglists and just retaining :arglists-str (or possibly renaming :arglists-str to :arglists). If a consumer needs to get at the data structure, they can call clojure.edn/read-string or something on it, if possible. If they can't parse the value of :arglists, they probably don't need it anyway.

[bbatsov]

Yeah, that's a good point. I've been using :arglists-str only as well in CIDER. Probably it's best to make both keys the same and add some notes explaining how this happened.