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

Error in :ret check when using with Orchestra 2020.07.12-1 #199

Closed
Limess opened this issue Aug 17, 2020 · 5 comments
Closed

Error in :ret check when using with Orchestra 2020.07.12-1 #199

Limess opened this issue Aug 17, 2020 · 5 comments

Comments

@Limess
Copy link

Limess commented Aug 17, 2020

Copied the details from a discussion on the Clojure slack, the messages in these links may expire.

First message around this issue: https://clojurians.slack.com/archives/CC68FBBAM/p1597150256011800
Follow up where we dug into it a little: https://clojurians.slack.com/archives/CC68FBBAM/p1597677078024100

I'm seeing the following exception:

java.lang.IllegalArgumentException: No matching clause: :clojure.spec.alpha/nil
	at expound.alpha$show_spec_name.invokeStatic(alpha.cljc:168)
	at expound.alpha$show_spec_name.invoke(alpha.cljc:165)
	at expound.alpha$eval18213$fn__18214.invoke(alpha.cljc:314)
	at clojure.lang.MultiFn.invoke(MultiFn.java:261)
	at expound.alpha$format_err.invokeStatic(alpha.cljc:335)
	at expound.alpha$format_err.invoke(alpha.cljc:331)
	at expound.alpha$eval18493$fn__18494.invoke(alpha.cljc:802)
	at clojure.lang.MultiFn.invoke(MultiFn.java:261)
	at expound.alpha$print_explain_data$iter__18500__18504$fn__18505$fn__18506.invoke(alpha.cljc:850)
	at expound.alpha$print_explain_data$iter__18500__18504$fn__18505.invoke(alpha.cljc:848)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:51)
	at clojure.lang.RT.seq(RT.java:535)
	at clojure.core$seq__5402.invokeStatic(core.clj:137)
	at clojure.core$apply.invokeStatic(core.clj:660)
	at clojure.core$apply.invoke(core.clj:660)
	at expound.alpha$print_explain_data.invokeStatic(alpha.cljc:847)
	at expound.alpha$print_explain_data.invoke(alpha.cljc:833)
	at expound.alpha$printer_str.invokeStatic(alpha.cljc:988)
	at expound.alpha$printer_str.invoke(alpha.cljc:970)
	at expound.alpha$custom_printer$fn__18569.invoke(alpha.cljc:1028)
	at org_api.middleware$wrap_exceptions$fn__28170.invoke(middleware.clj:53)
	at org_api.rollbar$wrap_exceptions$fn__23420.invoke(rollbar.clj:19)
	at org_api.middleware$wrap_unexpected_exceptions$fn__28180.invoke(middleware.clj:81)
	at ring.middleware.json$wrap_json_response$fn__6667.invoke(json.clj:139)
	at ring.middleware.keyword_params$wrap_keyword_params$fn__5554.invoke(keyword_params.clj:53)
	at ring.middleware.params$wrap_params$fn__4136.invoke(params.clj:67)
	at ring.middleware.absolute_redirects$wrap_absolute_redirects$fn__6013.invoke(absolute_redirects.clj:47)
	at ring.middleware.content_type$wrap_content_type$fn__5961.invoke(content_type.clj:34)
	at ring.middleware.default_charset$wrap_default_charset$fn__5985.invoke(default_charset.clj:31)
	at ring.middleware.not_modified$wrap_not_modified$fn__5942.invoke(not_modified.clj:61)
	at org_api.logging$middleware$fn__17647.invoke(logging.clj:8)
	at org_api.middleware$wrap_no_cache_param$fn__28187.invoke(middleware.clj:105)
	at opentracing_clj.ring$wrap_opentracing$fn__16360.invoke(ring.clj:45)
	at org_api.context$wrap_bind_user_id$fn__16381.invoke(context.clj:20)
	at iapetos.collector.ring$run_instrumented.invokeStatic(ring.clj:127)
	at iapetos.collector.ring$run_instrumented.invoke(ring.clj:123)
	at iapetos.collector.ring$wrap_instrumentation$fn__16161.invoke(ring.clj:163)
	at compojure.core$pre_init$fn__3997$fn__4000.invoke(core.clj:338)
	at compojure.core$wrap_route_middleware$fn__3878.invoke(core.clj:127)
	at compojure.core$wrap_route_info$fn__3883.invoke(core.clj:137)
	at compojure.core$wrap_route_matches$fn__3887.invoke(core.clj:146)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at compojure.core$make_context$handler__3974.invoke(core.clj:286)
	at compojure.core$make_context$fn__3978.invoke(core.clj:296)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at compojure.core$wrap_routes$fn__4007.invoke(core.clj:351)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at iapetos.collector.ring$wrap_metrics_expose$fn__16170.invoke(ring.clj:184)
	at clojure.lang.Var.invoke(Var.java:384)
	at ring.middleware.refresh$wrap_with_script$fn__4214.invoke(refresh.clj:80)
	at compojure.core$routing$fn__3902.invoke(core.clj:185)
	at clojure.core$some.invokeStatic(core.clj:2701)
	at clojure.core$some.invoke(core.clj:2692)
	at compojure.core$routing.invokeStatic(core.clj:185)
	at compojure.core$routing.doInvoke(core.clj:182)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:667)
	at clojure.core$apply.invoke(core.clj:660)
	at compojure.core$routes$fn__3906.invoke(core.clj:192)
	at ring.middleware.params$wrap_params$fn__4136.invoke(params.clj:67)
	at ring.middleware.reload$wrap_reload$fn__1837.invoke(reload.clj:39)
	at ring.middleware.stacktrace$wrap_stacktrace_log$fn__1209.invoke(stacktrace.clj:26)
	at ring.middleware.stacktrace$wrap_stacktrace_web$fn__1275.invoke(stacktrace.clj:98)
	at ring.adapter.jetty$proxy_handler$fn__484.invoke(jetty.clj:27)
	at ring.adapter.jetty.proxy$org.eclipse.jetty.server.handler.AbstractHandler$ff19274a.handle(Unknown Source)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.Server.handle(Server.java:500)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:383)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:547)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:375)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:273)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
	at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:129)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:375)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
	at java.base/java.lang.Thread.run(Thread.java:834)

when using expound with Orchestra:

[orchestra "2020.07.12-1"]
[expound "0.8.5"]

The explain data for this is:

{:clojure.spec.alpha/problems ({:path [:clojure.spec.alpha/nil], :pred nil?, :val {:email-domains [], :updated-at #inst "2020-08-17T14:57:04.174369000-00:00", :single-sign-on-service-url "REDACTED", :stage "DISABLED", :x509-public-certificate ".", :external-id "REDACTED", :id #uuid "2e6a3bea-eae5-4c47-8331-e0094c5bf106", :created-at #inst "2020-08-17T14:14:05.456667000-00:00", :organisation-ids []}, :via [], :in []} {:path [:clojure.spec.alpha/pred :enabled-idp :x509-public-certificate], :pred (clojure.core/complement org-api.spec.identity-provider/not-base-64?), :val ".", :via [:org-api.spec.identity-provider/enabled-identity-provider :org-api.spec.identity-provider/x509-public-certificate], :in [:x509-public-certificate]} {:path [:clojure.spec.alpha/pred :disabled-idp :x509-public-certificate], :pred (clojure.core/complement org-api.spec.identity-provider/not-base-64?), :val ".", :via [:org-api.spec.identity-provider/disabled-identity-provider :org-api.spec.identity-provider/x509-public-certificate], :in [:x509-public-certificate]}), :clojure.spec.alpha/spec #object[clojure.spec.alpha$nilable_impl$reify__2556 0xdebc6ea "clojure.spec.alpha$nilable_impl$reify__2556@debc6ea"], :clojure.spec.alpha/value {:email-domains [], :updated-at #inst "2020-08-17T14:57:04.174369000-00:00", :single-sign-on-service-url "REDACTED", :stage "DISABLED", :x509-public-certificate ".", :external-id "test", :id #uuid "2e6a3bea-eae5-4c47-8331-e0094c5bf106", :created-at #inst "2020-08-17T14:14:05.456667000-00:00", :organisation-ids []}, :clojure.spec.alpha/fn org-api.get-identity-provider/get-by-id, :clojure.spec.alpha/ret {:email-domains [], :updated-at #inst "2020-08-17T14:57:04.174369000-00:00", :single-sign-on-service-url "REDACTED", :stage "DISABLED", :x509-public-certificate ".", :external-id "REDACTED", :id #uuid "2e6a3bea-eae5-4c47-8331-e0094c5bf106", :created-at #inst "2020-08-17T14:14:05.456667000-00:00", :organisation-ids []}, :clojure.spec.alpha/failure :instrument, :orchestra.spec.test/caller {:file "identity_provider.clj", :line 238, :var-scope org-api.identity-provider/update!}}

and the failing spec is an object using s/keys, where the failing key is:

(defn not-base-64?
  "Returns true if a string is not base64 encoded.
   This does not mean the string _is correctly_ base 64 encoded."
  [s]
  (try
    (decode-base64 s)
    false
    (catch Exception _
      true)))

(s/def ::x509-public-certificate (s/and (complement string/blank?)
                                        (complement not-base-64?)))

When attempting to use stest/instrument instead I realised this was coming from the failure of :ret in an fdef. When calling a new function using the same spec as used in :ret, but instead in :args, expound worked as correctly.

I also tried removing the usage of s/and and this did not affect the failure.

The handler where we're using expound is as follows:

 (= :instrument (-> ex ex-data :clojure.spec.alpha/failure)) (do
                                                                          (try ((expound/custom-printer {:theme :figwheel-theme}) (-> ex ex-data))
                                                                               (catch Exception expound-ex
                                                                                 (log/errorf ex "Clojure spec instrumentation failure %s. Expound printing failed %s"
                                                                                             (-> ex ex-data) expound-ex)))

The above failures were when the spec definition for expound.alpha/value-in-context was overriden, as I'd previously seen this last week, but had written it off as user-error. The discussion led to the suggestion of disabling the spec for that function which was failing as follows (note that the actual spec causing the failure was different in the error below, but was the same failure mode):

2020-08-11 13:40:42.368 | ERROR | qtp225104551-20 | {:clojure.spec.alpha/problems [{:path [:spec-name :clojure.spec.alpha/pred], :pred #{:args :ret :fn :clojure.spec.alpha/pred}, :val :id, :via [], :in [1]} {:path [:spec-name :clojure.spec.alpha/nil], :pred nil?, :val :id, :via [], :in [1]}], :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__2509 0x7d665d19 clojure.spec.alpha$regex_spec_impl$reify__2509@7d665d19], :clojure.spec.alpha/value ({:show-valid-values? false, :print-specs? true, :theme :figwheel-theme} :id nil [] nil), :clojure.spec.alpha/fn expound.alpha/value-in-context, :clojure.spec.alpha/args ({:show-valid-values? false, :print-specs? true, :theme :figwheel-theme} :id nil [] nil), :clojure.spec.alpha/failure :instrument, :orchestra.spec.test/caller {:file core.clj, :line 673, :var-scope clojure.core/apply}}
clojure.lang.ExceptionInfo: Call to #'expound.alpha/value-in-context did not conform to spec.
@bhb
Copy link
Owner

bhb commented Sep 30, 2020

@Limess Thanks for reporting this and apologies for the long delay. It appears the new implementation of Orchestra had some changes in the explain-data that broken Expound and it took me a bit to figure out how to support both the old and new formats.

Do you have the ability to try out a specific git SHA in your project? If so, I'd appreciate it if you could try cdabbff before I release it. I think it may solve your problem, but frankly I'm not 100% sure. I tried to construct a repro based on your description above but I may be missing something crucial. Thanks!!

@Limess
Copy link
Author

Limess commented Oct 1, 2020

No worries, really appreciate your work on this!

I checked out that SHA and tried it and it fixes the issue we were seeing 👍🏻.

@bhb
Copy link
Owner

bhb commented Oct 1, 2020

@Limess Thanks for trying it out!

@bhb
Copy link
Owner

bhb commented Oct 1, 2020

Fixed in #202

@bhb
Copy link
Owner

bhb commented Oct 2, 2020

@Limess Thanks again for your help. I've just released 0.8.6, which includes this fix.

@bhb bhb closed this as completed Oct 2, 2020
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

2 participants