spec passing nil as second argument to `ExceptionInfo` constructor...

Question

Exception thrown from this line:
https://github.com/clojure/clojure/blob/clojure-1.9.0/src/jvm/clojure/lang/ExceptionInfo.java#L31

Due to this call:
https://github.com/clojure/spec.alpha/blob/spec.alpha-0.1.143/src/main/clojure/clojure/spec/test/alpha.clj#L279

if when-not returns nil, ExceptionInfo throws exception on line 31.

A naive fix may be:
(apply ex-info (remove nil? (link: "Specification-based check failed" (when-not ...))))

although that is really just masking over the issue and provides no actionable info to the sufferer of the failure.

Unfortunately I have no minimal test case as this is proprietary (and complicated code).

Perhaps the author from reading the code will have more insight.

Here's the full stack trace:

(link: [clojure.lang.ExceptionInfo "ExceptionInfo.java" 31) (link: clojure.lang.ExceptionInfo "ExceptionInfo.java" 22) (link: clojure.core$ex_info invokeStatic "core.clj" 4739) (link: clojure.core$ex_info invoke "core.clj" 4739) (link: clojure.spec.test.alpha$explain_check invokeStatic "alpha.clj" 277) (link: clojure.spec.test.alpha$explain_check invoke "alpha.clj" 275) (link: clojure.spec.test.alpha$check_call invokeStatic "alpha.clj" 295) (link: clojure.spec.test.alpha$check_call invoke "alpha.clj" 285) (link: clojure.spec.test.alpha$quick_check$fn__2986 invoke "alpha.clj" 308) (link: clojure.lang.AFn applyToHelper "AFn.java" 154) (link: clojure.lang.AFn applyTo "AFn.java" 144) (link: clojure.core$apply invokeStatic "core.clj" 657) (link: clojure.core$apply invoke "core.clj" 652) (link: clojure.test.check.properties$apply_gen$fn__16139$fn__16140 invoke "properties.cljc" 30) (link: clojure.test.check.properties$apply_gen$fn__16139 invoke "properties.cljc" 29) (link: clojure.test.check.rose_tree$fmap invokeStatic "rose_tree.cljc" 77) (link: clojure.test.check.rose_tree$fmap invoke "rose_tree.cljc" 73) (link: clojure.test.check.generators$fmap$fn__9199 invoke "generators.cljc" 101) (link: clojure.test.check.generators$gen_fmap$fn__9173 invoke "generators.cljc" 57) (link: clojure.test.check.generators$call_gen invokeStatic "generators.cljc" 41) (link: clojure.test.check.generators$call_gen invoke "generators.cljc" 37) (link: clojure.test.check$quick_check invokeStatic "check.cljc" 94) (link: clojure.test.check$quick_check doInvoke "check.cljc" 37) (link: clojure.lang.RestFn invoke "RestFn.java" 425) (link: clojure.lang.AFn applyToHelper "AFn.java" 156) (link: clojure.lang.RestFn applyTo "RestFn.java" 132) (link: clojure.core$apply invokeStatic "core.clj" 657) (link: clojure.core$apply invoke "core.clj" 652) (link: clojure.spec.gen.alpha$quick_check invokeStatic "alpha.clj" 29) (link: clojure.spec.gen.alpha$quick_check doInvoke "alpha.clj" 27) (link: clojure.lang.RestFn applyTo "RestFn.java" 137) (link: clojure.core$apply invokeStatic "core.clj" 661) (link: clojure.core$apply invoke "core.clj" 652) (link: clojure.spec.test.alpha$quick_check invokeStatic "alpha.clj" 309) (link: clojure.spec.test.alpha$quick_check invoke "alpha.clj" 302) (link: clojure.spec.test.alpha$check_1 invokeStatic "alpha.clj" 335) (link: clojure.spec.test.alpha$check_1 invoke "alpha.clj" 323) (link: clojure.spec.test.alpha$check$fn__3005 invoke "alpha.clj" 411) (link: clojure.core$pmap$fn__8105$fn__8106 invoke "core.clj" 6942) (link: clojure.core$binding_conveyor_fn$fn__5476 invoke "core.clj" 2022) (link: clojure.lang.AFn call "AFn.java" 18) (link: java.util.concurrent.FutureTask run "FutureTask.java" 266) (link: java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1149) (link: java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 624) (link: java.lang.Thread run "Thread.java" 748)]

Answer 1

Comment made by: johanatan

So, I was able to finally track this down to an actual problem where my function value returned from the function under test would throw and exception on a particular input.

This patch helps point people in that direction (although an even better one would be to surface/explain the underlying failure rather than just allude to it).

Answer 2

Comment made by: johanatan

Anyone going to respond to this? What's the usual expected response time for feedback on a patch/pull request?

Answer 3

Comment made by: alexmiller

There is not enough information here for this ticket to be actionable. If (s/valid? spec data) is false, then (s/explain-data spec data) should never be nil. The fact that you're seeing that is indication of a bug in the spec implementation, but without more info about the spec or the data, there's nothing I can do at the moment.

Answer 4

Comment made by: johanatan

The "actionable" part is the submitted patch. This code is more resilient and friendly to sufferers of the issue.

The actual text of this ticket is merely a CLONE of the original (since I could not re-open or edit the original).

Answer 5

Comment made by: johanatan

i.e., the only way I could submit this patch was to create a CLONE.

Answer 6

Comment made by: jafingerhut

Jonathan, since you are on the list of Clojure contributors, I have bumped up your permissions on Clojure JIRA so you should be able to edit tickets now.

I do not think there is a 'usual time' by which tickets are responded to. It can vary by orders of magnitude, depending upon clarity and perceived criticality of the issue.

Alex would probably appreciate if the description included an example that can be evaluated in order to reproduce the problem. Often by seeing the problem, they may consider other solutions to the problem.

Answer 7

Comment made by: alexmiller

The patch is addressing the symptom, not the problem. We really need the spec and data value causing the issue.

Answer 8

Comment made by: johanatan

The patch does exactly what it claims to do: provide better messaging for this scenario.

Unfortunately my attempt at constructing a minimal repro from scratch was not successful. I understand that you would like me to do additional work but all work that I've done up to this point is pro bono and I cannot donate any more of my time to this cause at this time.

We should discuss the merits of this patch on its own-- is it an improvement over the previous code or not? (link: And static human analysis of the code can determine that it is in fact an improvement). The root "problem" here is that s/valid? and s/explain-data are not pure, total functions-- they can fail probabilistically depending on the arguments (i.e., when there is a higher-order function involved (link: which due to a fundamental limitation of spec can only be "verified" by passing random data through it)). In fact, I cannot imagine any better way to address this issue given that limitation (except bubbling up the exception which was certainly swallowed in my original, real-world reproduction of this but which was not in my attempt at the minimal repro).

Answer 9

Comment made by: johanatan

I would appreciate a response to the provided reasoning.

Answer 10

Comment made by: gshayban

Without a reproduction case, I'm afraid this will not move forward. If there is an invariant being broken (e.g. invalid data but missing explain-data) provide a example. It doesn't have to be minimal, just needs to be reproducible.

(link: 1) https://dev.clojure.org/display/community/Creating Tickets
(link: 2) https://dev.clojure.org/display/community/Developing Patches (see Before You Code)

Answer 11

Comment made by: johanatan

So, are you saying that the Clojure project does not accept bug reports that were discovered by manual human inspection? Because if it does accept such, then you should consider this bug to be a case of such.

Also, if you would allow yourselves to merely think about the two functions in question, you can "discover" the bug for yourselves. Do you accept that s/explain-data and s/valid? are not purely deterministic? i.e., they can return different results than both each other and themselves given the exact same input on two successive runs. If you do accept this, then you have convinced yourselves of the validity of this bug claim (and I strongly urge you to accept that because it is the reality of the situation).

Answer 12

Comment made by: gshayban

I am trying to help you formulate a better, actionable ticket. I don't speak on behalf of 'the Clojure project'. I implore you to tone down the antagonism. Language like "if you would allow yourselves to merely think about the two functions in question" doesn't belong here.

I don't doubt that you've uncovered an issue, but as it stands this is unactionable.

Answer 13

Comment made by: johanatan

I think you should go back and re-read my previous message. Imagine that it were spoken calmly, plainly and matter-of-factly because that is how it was written/intended (instead of however you have chosen to read it). Also, it was not new information— merely a rewording of previous communications (which do not seem to have been understood).

And I really do not appreciate the sanctimonious, condescending, patronizing in your previous message to me. Save the moral grandstanding for somewhere more appropriate (wherever that may be). It has no place in a technical bug report.

Answer 14

Comment made by: alexmiller

Regarding the non-determinism, that's a known issue being tracked in CLJ-1949. Assuming that's addressed, I don't see the need for this change. However, I'm not sure where all that would go so I'm just going to leave this ticket open and defer to later when that becomes clearer.

Answer 15

Comment made by: johanatan

Ah, yes, if CLJ-1949 were fixed then this would not be an issue. Thanks for the (reasonable) explanation.