Feature request: No Exceptions/Errors on failures

[LispEngineer]

Hi there,

First off - thank you for this. I know Alex Miller and Rich Hickey are very opinionated (as is their right of course) about how Clojure and Spec are supposed to be used and indeed removed the :fn and :ret checking from the earlier versions of fdef instrumentation. I'm extremely happy you have added it back, as I love to be able to have multiple different kinds of testing (predefined unit, generational, and this sort of last chance monitoring of the running system).

I'd love to suggest an improvement for your consideration please.

Allow for an orchestra function to be defined which, if non-nil, will be called when an instrumented/fdef function's assertions would fail, would call this function with details of the failure instead of throwing an AssertionError. You might consider a global for the entire application, or a dynamic variable which can be used in certain sensitive areas, or even a different one for each fdef spec, stored in a map, etc. But, for the initial try, I'd be entirely happy with a single global function.

I looked at the code and there didn't seem to be too many places you throw things, so hopefully it would be straight-forward to implement.

This would allow the fdef instrumented assertion failures to be used and handled in whatever way suits the application, and would even, for example (assuming the overhead was acceptable), allow them to be used in production.

One thought was that I would love to deploy new code to production with fdef instrumentation turned on but the failures logged to an error log for a few days to ensure that there are no unexpected problems logged. That is much more gentle than breaking the application with a thrown error.

It would also be neat to be able to turn off the checking of instrumented functions with another global variable at runtime. This way I could connect to my running server over nREPL (yes, I run them in production that way), set the "assertion failure handling function", then set the "enable fdef spec checking" and find out more information about any production problems in a running system.

The function could do anything, including keeping a tab on the rate of failures and escalating things if the rate is high, etc.

Anyway, just a thought that I feel would add enormous flexibility to your Orchestra.

Thanks!

[jeaye]

Hey there, thanks for opening this up for discussion and I'm glad you've found orchestra to be useful. This is a good suggestion and I'm up for implementing it, but it'll likely need to wait a couple of weeks, since I'm currently very close to a delivery at work. You did bring up a couple of points though, so I'll try to address each of them.

Allow for an orchestra function to be defined which, if non-nil, will be called when an instrumented/fdef function's assertions would fail, would call this function with details of the failure instead of throwing an AssertionError.

Absolutely. I think we can follow expound in this behavior without much effort.

It would also be neat to be able to turn off the checking of instrumented functions with another global variable at runtime.

Arguably, this can be done with unstrument to simply remove all instrumented symbols. It could also be done with some logic in the custom failure handler, but I think we'll agree that's extra processing that we wouldn't want done. There is also with-instrument-disabled for handling this situation locally, in case you hadn't yet seen it. If none of these work for you, I'd still be open to adding global configuration for this as well.

Thanks again for the detailed request; I think this'll certainly make orchestra better.

[LispEngineer]

Hi again. I waited before responding as you said you'd be busy several weeks and I didn't want to seem like I was putting any pressure upon you and your volunteer time.

I think the instrument, unstrument would be fine, especially as I run an nREPL on a port on my servers for live interaction with the running server. Are these namespace sensitive? Nothing in reading the docstring (or glancing at the code) seems to indicate that. It would be nice to be able to say (e.g., with a runtime flag or configuration) something like (instrument-ns 'myapp.core 'myapp.util 'myapp.xyz) and the equivalent unstrument-ns. Or, heck, I don't know if there is some sort of namespace globbing library: (instrument 'myapp.*).

One last comment: https://github.com/tonsky/clojure-future-spec is used to make clojure.future.spec work in Clojure 1.8 environments. Any thoughts on orchestra and compatibility with 1.8?

Thanks again!