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

With cider-20140528.928, decode errors on every operation #588

Closed
hugoduncan opened this issue May 29, 2014 · 18 comments
Closed

With cider-20140528.928, decode errors on every operation #588

hugoduncan opened this issue May 29, 2014 · 18 comments
Milestone

Comments

@hugoduncan
Copy link
Member

With todays cider from melpa, the info op on some symbols seems to put cider into a state where all further operations result in an error.

(error "Cannot decode object: 3083")

The actual number seems to be monotonically increasing (but non-consecutive) with each operation.

@hugoduncan
Copy link
Member Author

The doc string for one such problematic symbol:

(doc pallet.plan/execute-plan)
-------------------------
pallet.plan/execute-plan
([session target plan-fn])
  Using the session, execute plan-fn on target. Uses any plan
  middleware defined on the plan-fn.  Results are recorded by any
  recorder in the session, as well as being returned.

## Function Signatures

  - BaseSession Target PlanFn -> (maybe PlanTargetResult)

@hugoduncan
Copy link
Member Author

Looks like the info op returns the full metadata of a var, so here is the metadata for the above function. I suspect the issue is caused by the :sig metadata. Looking at cider.nrepl.middleware.info/format-response suggests it might not be very robust to arbitrary metadata on vars.

{:ns #<Namespace pallet.plan>,
 :name execute-plan,
 :file "pallet/plan.clj",
 :column 1,
 :line 190,
 :arglists ([session target plan-fn]),
 :doc
 "Using the session, execute plan-fn on target. Uses any plan\n  middleware defined on the plan-fn.  Results are recorded by any\n  recorder in the session, as well as being returned.\n\n## Function Signatures\n\n  - BaseSession Target PlanFn -> (maybe PlanTargetResult)",
 :sig
 [[{:execution-state
    {:executor pallet.core.executor.protocols.ActionExecutor,
     {:k :recorder} pallet.core.recorder.protocols.Record,
     {:k :action-options}
     {{:p? #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
       :pred-name keyword?}
      {:_ nil}},
     {:k :user} {:_ nil},
     {:k :environment} {:_ nil},
     {:k :extension}
     {{:p? #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
       :pred-name keyword?}
      {:_ nil}},
     {:k :record-all}
     {{:p? #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
       :pred-name keyword?}
      java.lang.Boolean},
     {:k :event-fn} {:_ nil}},
    {:k :plan-state} pallet.core.plan_state.protocols.StateGet,
    :type {:v :pallet.session/session}}
   {{:p? #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
     :pred-name keyword?}
    {:_ nil}}
   {:output-schema {:_ nil},
    :input-schemas
    ([{:schema
       {:execution-state
        {:executor pallet.core.executor.protocols.ActionExecutor,
         {:k :recorder} pallet.core.recorder.protocols.Record,
         {:k :action-options}
         {{:p?
           #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
           :pred-name keyword?}
          {:_ nil}},
         {:k :user} {:_ nil},
         {:k :environment} {:_ nil},
         {:k :extension}
         {{:p?
           #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
           :pred-name keyword?}
          {:_ nil}},
         {:k :record-all}
         {{:p?
           #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
           :pred-name keyword?}
          java.lang.Boolean},
         {:k :event-fn} {:_ nil}},
        {:k :plan-state} pallet.core.plan_state.protocols.StateGet,
        :type {:v :pallet.session/session}},
       :optional? false,
       :name arg0}
      {:schema {:_ nil}, :optional? false, :name arg1}])}
   :-
   {:schema
    {:target
     {{:p? #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
       :pred-name keyword?}
      {:_ nil}},
     {:k :action-results}
     [{{:k :error} {:_ nil},
       {:k :summary} {:schema java.lang.String},
       {:p?
        #<core$keyword_QMARK_ clojure.core$keyword_QMARK_@33a42080>,
        :pred-name keyword?}
       {:_ nil}}],
     {:k :return-value} {:_ nil},
     {:k :exception} java.lang.Exception}}]]}

@bbatsov
Copy link
Member

bbatsov commented May 29, 2014

@hugoduncan Seems to me you're correct. It seem we'll have to do whitelisting of attributes allowed in the response (as opposed to the current blacklist approach). //cc @gtrak @jeffvalk

@bbatsov
Copy link
Member

bbatsov commented May 29, 2014

@hugoduncan Btw, the number if the error is the point's position in the connection buffer (responses are inserted in the connection buffer, decoded and deleted; if something goes wrong the error reports the current position where something went wrong). I'll replace this with something more sensible. The encode/decode code needs a lot of love...

@gtrak
Copy link
Contributor

gtrak commented May 29, 2014

The first thing I tried was a whitelist, unfortunately it excluded some
important things, so it got reverted.

We could make it a 'little' more robust, by simply excluding keys
dynamically that cannot be encoded.

On Thu, May 29, 2014 at 7:24 AM, Bozhidar Batsov
[email protected]:

@hugoduncan https://github.com/hugoduncan Btw, the number if the error
is the point's position in the connection buffer (responses are inserted in
the connection buffer, decoded and deleted; if something goes wrong the
error reports the current position where something went wrong). I'll
replace this with something more sensible. The encode/decode code needs a
lot of love...


Reply to this email directly or view it on GitHubhttps://github.com//issues/588#issuecomment-44522522
.

@jeffvalk
Copy link
Contributor

The whitelist approach would force us, at least in a trivial sense, to define/document a schema for info. This is probably good thing.

@gtrak Your original idea made good sense; if we can coordinate on keys, I certainly vote for this.

@bbatsov
Copy link
Member

bbatsov commented May 29, 2014

👍 for a whitelist, no matter how long it would be.

@gtrak
Copy link
Contributor

gtrak commented May 29, 2014

Since this is arbitrary metadata, there's still no guarantee that the user
or some framework won't somehow pollute one of our expected keys.

In addition to the whitelist, I think I should still do the dissoc'ing
silent-failure at least for the top-level map (maybe we add a backchannel
for such errors).

On Thu, May 29, 2014 at 9:24 AM, Bozhidar Batsov
[email protected]:

[image: 👍] for a whitelist, no matter how long it would be.


Reply to this email directly or view it on GitHubhttps://github.com//issues/588#issuecomment-44530658
.

@tbatchelli
Copy link

+1 on silently dissoc'ing the troublesome entries. When this fails it leaves my emacs/cider unusable until I restart the JVM process.

@bbatsov
Copy link
Member

bbatsov commented May 30, 2014

@gtrak @jeffvalk We should also add some check for incomplete messages (as nREPL streams the responses in chunks of 4k I think). Obviously we need a way to tell apart an incomplete message from a corrupted message. The lack of such mechanism got us into this predicament in the first place (the old code simply assumed that every decode error is an incomplete message).

@gtrak
Copy link
Contributor

gtrak commented May 30, 2014

Time to implement core.async on elisp! ;-)

On Friday, May 30, 2014, Bozhidar Batsov [email protected] wrote:

@gtrak https://github.com/gtrak @jeffvalk https://github.com/jeffvalk
We should also add some check for incomplete messages (as nREPL streams the
responses in chunks of 4k I think). Obviously we need a way to tell apart
an incomplete message from a corrupted message. The lack of such mechanism
got us into this predicament in the first place (the old code simply
assumed that every decode error is an incomplete message).


Reply to this email directly or view it on GitHub
#588 (comment).

@ghost
Copy link

ghost commented Jun 10, 2014

@gtrak I think an error I'm having may be related? Getting this on emacs GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.12.1) of 2014-05-14 on trouble, modified by Debian when calling cider-jack-in while having a file open in a leiningen project. Versions of CIDER related stuff is in the nrepl_decode function call.

(error "Cannot decode object: 1")
  signal(error ("Cannot decode object: 1"))
  error("Cannot decode object: %d" 1)
  nrepl-bdecode-buffer()
  nrepl-decode(#("; CIDER 0.7.0alpha (package: 20140610.608) (Java 1.7.0_55, Clojure 1.6.0, nREPL 0.2.3, cider-nrepl 0.7.0-snapshot)\nlearning.core> " 0 114 (face font-lock-comment-face fontified nil) 114 115 (fontified nil) 115 130 (fontified nil face cider-repl-prompt-face read-only t intangible t cider-prompt t rear-nonsticky (cider-prompt read-only face intangible))))
  nrepl-net-decode()
  #[0 "\300 \301V\205 

This appears to causes REPL error messages not to show up anywhere I can find them, with no other negative effects I have noticed (eldoc works, for instance). Output from valid code still shows up in the repl buffer.

In addition, starting the REPL with cider-jack-in again gives me just an nrepl buffer, not a cider buffer and no error message. (As in the comment header is "; nrepl.el 0.2.0 (Clojure 1.6.0, nREPL 0.2.3)" with buffer name "nrepl learning-clojure" instead of "; CIDER 0.7.0alpha (package: 20140610.608) (Java 1.7.0_55, Clojure 1.6.0, nREPL 0.2.3, cider-nrepl 0.7.0-snapshot)" with buffer name "cider-repl learning-clojure")

@gtrak
Copy link
Contributor

gtrak commented Jun 10, 2014

I think there are some edge-cases related to running cider-nrepl outside a
proper lein project, is that the case here?

On Tue, Jun 10, 2014 at 4:15 PM, Chris Carpenter [email protected]
wrote:

@gtrak https://github.com/gtrak I think an error I'm having may be
related? Getting this on emacs GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+
Version 3.12.1) of 2014-05-14 on trouble, modified by Debian when calling
cider-jack-in while having a file open in a leiningen project. Versions of
CIDER related stuff is in the nrepl_decode function call.

(error "Cannot decode object: 1")
signal(error ("Cannot decode object: 1"))
error("Cannot decode object: %d" 1)
nrepl-bdecode-buffer()
nrepl-decode(#("; CIDER 0.7.0alpha (package: 20140610.608) (Java 1.7.0_55, Clojure 1.6.0, nREPL 0.2.3, cider-nrepl 0.7.0-snapshot)\nlearning.core> " 0 114 (face font-lock-comment-face fontified nil) 114 115 (fontified nil) 115 130 (fontified nil face cider-repl-prompt-face read-only t intangible t cider-prompt t rear-nonsticky (cider-prompt read-only face intangible))))
nrepl-net-decode()
#[0 "\300 \301V\205

This appears to causes REPL error messages not to show up anywhere I can
find them, with no other negative effects I have noticed (eldoc works, for
instance). Output from valid code still shows up in the repl buffer.

In addition, starting the REPL with cider-jack-in again gives me just an
nrepl buffer, not a cider buffer and no error message. (As in the comment
header is "; nrepl.el 0.2.0 (Clojure 1.6.0, nREPL 0.2.3)" with buffer name
"nrepl learning-clojure" instead of "; CIDER 0.7.0alpha (package:
20140610.608) (Java 1.7.0_55, Clojure 1.6.0, nREPL 0.2.3, cider-nrepl
0.7.0-snapshot)" with buffer name "cider-repl learning-clojure")


Reply to this email directly or view it on GitHub
#588 (comment).

@ghost
Copy link

ghost commented Jun 11, 2014

@gtrak Well, it is in a lein project. I can't promise my settings are correct. I've got a global profiles.clj that says:

{:user {:dependencies [[org.clojure/clojure "1.6.0"]]
        :plugins [[cider/cider-nrepl "0.7.0-SNAPSHOT"]]}}

Also, my lein project.clj is:

(defproject learning "0.1.0-SNAPSHOT"
  :description "Repo to hold code generated while learning clojure"
  :url "http://github.com/mordocai/learning-clojure/"
  :license {:name "BSD 3-Clause License"
            :url "http://opensource.org/licenses/BSD-3-Clause"}
  :dependencies [[org.clojure/clojure "1.6.0"]]
  :main ^:skip-aot learning.core
  :target-path "target/%s"
  :profiles {:uberjar {:aot :all}})

@bbatsov
Copy link
Member

bbatsov commented Jun 11, 2014

@mordocai Seems you have nrepl.el installed as well. You have to remove it, as it cannot co-exist with cider.

@ghost
Copy link

ghost commented Jun 11, 2014

@bbatsov @gtrak
Excellent, I was somehow under the impression that nrepl was required for cider :(. Thank you, looks like it is working much better now! Still getting an error message, but doesn't appear to hurt anything.

Error: (error "Cannot decode message: ; CIDER 0.7.0alpha (package: 20140611.315) (Java 1.7.0_55, Clojure 1.6.0, nREPL 0.2.3, cider-nrepl 0.7.0-snapshot)
learning.core> ")

Does emacs have a way of specifying conflicting packages or is it possible to de-conflict nrepl from cider? One or the other should probably be done, I doubt i'm the only one who will run into this.

@bbatsov
Copy link
Member

bbatsov commented Jun 12, 2014

@mordocai That's a known issue - #583.

@bbatsov bbatsov closed this as completed Jun 12, 2014
@bbatsov
Copy link
Member

bbatsov commented Jun 12, 2014

Does emacs have a way of specifying conflicting packages or is it possible to de-conflict nrepl from cider? One or the other should probably be done, I doubt i'm the only one who will run into this.

No, there's not such mechanism in package.el. nrepl.el was cider's old name; it's mentioned in the README users are supposed to remove nrepl.el before installing cider. While we can do a check for nrepl.el explicitly and warn the users, the final nrepl.el release is about an year old and I doubt many people are still using it.

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

No branches or pull requests

5 participants