Can't get stacktrace for exception in test report

[kommen]

Expected behavior

Clicking an error in a test report should show the stacktrace for that error.

Actual behavior

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  nrepl-send-request(("op" "test-stacktrace" "ns" "com.nextjournal.graph-test" "var" "normalize-test" "index" 1 "pprint-fn" "cider.nrepl.pprint/pprint" "print-options" (dict "length" 50 "level" 50)) (closure ((causes) (index . 1) (var . "normalize-test") (ns . "com.nextjournal.graph-test") t) (response) (let ((class (nrepl-dict-get response "class")) (status (nrepl-dict-get response "status"))) (cond (class (setq causes (cons response causes))) (status (if causes (progn (cider-stacktrace-render ... ...))))))) nil)
  cider-nrepl-send-request(("op" "test-stacktrace" "ns" "com.nextjournal.graph-test" "var" "normalize-test" "index" 1 "pprint-fn" "cider.nrepl.pprint/pprint" "print-options" (dict "length" 50 "level" 50)) (closure ((causes) (index . 1) (var . "normalize-test") (ns . "com.nextjournal.graph-test") t) (response) (let ((class (nrepl-dict-get response "class")) (status (nrepl-dict-get response "status"))) (cond (class (setq causes (cons response causes))) (status (if causes (progn (cider-stacktrace-render ... ...))))))) nil)
  (let (causes) (cider-nrepl-send-request (nconc (list "op" "test-stacktrace" "ns" ns "var" var "index" index) (if (cider--pprint-fn) (progn (list "pprint-fn" (cider--pprint-fn)))) (if cider-stacktrace-print-options (progn (list "print-options" cider-stacktrace-print-options)))) (function (lambda (response) (let ((class (nrepl-dict-get response "class")) (status (nrepl-dict-get response "status"))) (cond (class (setq causes ...)) (status (if causes ...)))))) (cider-current-repl)))
  cider-test-stacktrace-for("com.nextjournal.graph-test" "normalize-test" 1)
  cider-test-stacktrace()
  (lambda (_button) (cider-test-stacktrace))(#<marker (moves after insertion) at 255 in *cider-test-report*>)
  button-activate(#<marker (moves after insertion) at 255 in *cider-test-report*> t)
  push-button(255 t)
  push-button((mouse-2 (#<window 7 on *Backtrace*> 255 (178 . 247) 17298417 nil 255 (25 . 15) nil (3 . 7) (7 . 16))))
  funcall-interactively(push-button (mouse-2 (#<window 7 on *Backtrace*> 255 (178 . 247) 17298417 nil 255 (25 . 15) nil (3 . 7) (7 . 16))))
  call-interactively(push-button nil nil)
  command-execute(push-button)

Steps to reproduce the problem

1. produce an error in a test
2. click the link to "View causes and stacktrace" in the test report

Environment & Version information

CIDER version information

;; CIDER 0.20.0 (package: 20190115.956) (Oslo), nREPL 0.5.3
;; Clojure 1.10.0, Java 1.8.0_102

Emacs version

GNU Emacs 26.1.91 (build 2, x86_64-apple-darwin18.2.0, Carbon Version 158 AppKit 1671.2) of 2019-01-10

Operating system

macOS 10.14.3 (18D109)

[kommen]

I've tracked this down to a problem in the sesman-friendly-session-p implementation for cider:

file in sesman-friendly-session-p for the test report buffer is set to the project root directory. this directory is neither listed in classpath nor in classpath-roots. Thus sesman-friendly-session-p returns nil, causing cider-repls for the test report buffer to also turn out empty, even if there are repls connected. This eventually leads cider-current-repl to return nil, which is passed to cider-nrepl-send-request in the stack trace posted above, causing the error.

cc @vspinu

[kommen]

I'm able to work around this by manually linking the *cider-test-report* buffer to the cider session via sesman-link-with-buffer.

[kommen]

Digging more into this, it shows I was led down the wrong path: sesman-friendly-session-p is not the issue here.

For some reason, my *cider-test-report* buffer's default-directory doesn't end with an /. While the project link value ends with an /. This let's the string-match-p fail here:
https://github.com/vspinu/sesman/blob/966c13812f312c748ef984d16ce4cefe0df0f5c9/sesman.el#L975-L979

Wrapping (sesman-expand-path default-directory) in an file-name-as-directory fixes the issue for me, but it still isn't clear to me why default-directory is seemingly incorrect in the first place.

[kommen]

Well, it was our own tooling, as we passed the shell's pwd to cider-connect-clj&cljs as the :project-dir, which didn't have the trailing slash.

@bbatsov @vspinu What do you think about normalizing that in cider--update-project-dir to save others from this?

[vspinu]

Thanks @kommen for digging all of this. I think your proposal makes a lot of sense.

[vspinu]

You had two proposals here. I was referring to the change on the sesman side. Or do you now believe it's not the right place to address it?

[kommen]

@vspinu from my current point of view, only cider's update-project-dir needs to be changed so that if it is passed a dir name without a trailing slash to add one.

Because the project dir is used to set *cider-test-report* buffer's default-directory, which the emacs docs describe:

default-directory is a variable defined in ‘C source code’.
Documentation:
Name of default directory of current buffer.
It should be a directory name (as opposed to a directory file-name).
On GNU and Unix systems, directory names end in a slash ‘/’.
To interactively change the default directory, use command ‘cd’.

When later cider tries to find the repl for the *cider-test-report* buffer, the missing slash let's this check in sesman-relevant-context-p fail.

So, to be extra safe, sesman-relevant-context-p could be adapted to consider dir names where only the trailing slash is present or not the same, but I don't think this is a requirement.

[kommen]

I do think I have another proposal, but also for cider: The original error message is not very helpful. It would be better to get something that no repl was found instead. This can probably be addressed passing 'ensure in the right calls to cider-current-repl at the correct call sites. I will file an issue or PR for that soon.

[vspinu]

So, to be extra safe, sesman-relevant-context-p could be adapted to consider dir names where only the trailing slash is present or not the same, but I don't think this is a requirement.

I have already made a change to sesman with stripping extra slash in sesman-expand-path. The entire logic should work on symllinks to directories and those don't have the trailing slash.

I don't think the change to cider is necessary. I think the consumers of default-directory should not assume trailing slash.

The original error message is not very helpful

Yes. Have you tracked why the error is the way it is? Would be great if you could provide a patch for this and maybe other similar cases. I am on a long trip to China and cannot look at it right now.

[kommen]

Yes. Have you tracked why the error is the way it is?

yes, I did. I think I can provide a patch.

[kommen]

@vspinu I can confirm that with vspinu/sesman@3df3301 my original issue is fixed. Thank you!

I've submitted #2591 for the better error messages as a follow up.