[DRAFT] Fix RPC handler when lazy response data throws an error #129
Open
sundbry wants to merge 2 commits intoprotojure:masterfrom
Open
[DRAFT] Fix RPC handler when lazy response data throws an error #129sundbry wants to merge 2 commits intoprotojure:masterfrom
sundbry wants to merge 2 commits intoprotojure:masterfrom
Conversation
The `protos` target should generate code under test/test/ instead of just test/ Signed-off-by: Ryan Sundberg <ryan@arctype.co>
This reproduces a bug where an exception raised in a lazy sequence attached to a protobuf response causes the request handler to crash and time out. Signed-off-by: Ryan Sundberg <ryan@arctype.co>
Member
|
Interesting find. Ill try to dig in and see if I can come up with a solution. BTW: the linter errors can be fixed with 'lein cljfmt fix'. The protoc-gen-clojure plugin doesn't output code that passes cljfmt, unfortunately, so this step is usually necessary when re-generating the test protos. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reproduces a bug where an exception raised in a lazy sequence attached to a protobuf response causes the request handler to crash and time out.
The test case does not pass. I am not sure how to approach fixing this yet. The obvious workaround is to just not use lazy sequences (
mapvinstead ofmapetc). Probably if we can just catch the error in the right place it will fix this.The core of the issue is that when a lazy seq is returned, the exception does not raise in the wrapped handler function. It stays unresolved until it gets evaluated in the serializer in
grpc/codec/lpm.cljand then bubbles up into the grpc-leave interceptor.Test log with pedestal debugging
DEBUG 2022-03-19 00:52:06,606 io.pedestal.interceptor.chain: {:in process-all, :handling :enter, :execution-id 2, :line 154} DEBUG 2022-03-19 00:52:06,606 io.pedestal.interceptor.chain: {:interceptor :protojure.test.grpc.TestService/ShouldThrow-handler, :stage :enter, :execution-id 2, :fn #object[protojure.pedestal.routes$handler$fn__26079 0x8d57de5 "protojure.p edestal.routes$handler$fn__26079@8d57de5"], :line 50} DEBUG 2022-03-19 00:52:06,606 io.pedestal.interceptor.chain: {:in leave-all, :execution-id 2, :line 241} DEBUG 2022-03-19 00:52:06,607 io.pedestal.interceptor.chain: {:interceptor :protojure.pedestal.interceptors.grpc/interceptor, :stage :leave, :execution-id 2, :fn #object[clojure.core$partial$fn__5857 0x675f76b5 "clojure.core$partial$fn__58 57@675f76b5"], :line 50} ERROR 2022-03-19 00:52:06,609 protojure.pedestal.interceptors.grpc: {:msg "Pipeline", :line 117} clojure.lang.ExceptionInfo: This is also supposed to fail (2) {} at protojure.grpc_test.TestService$fn__31646.invoke(grpc_test.clj:199) at clojure.core$map$fn__5884.invoke(core.clj:2757) 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__5419.invokeStatic(core.clj:139) at clojure.core$seq__5419.invoke(core.clj:139) at clojure.spec.alpha$every_impl$reify__2255.conform_STAR_(alpha.clj:1317) at clojure.spec.alpha$conform.invokeStatic(alpha.clj:171) at clojure.spec.alpha$conform.invoke(alpha.clj:167) at clojure.spec.alpha$map_spec_impl$reify__1998.conform_STAR_(alpha.clj:844) at clojure.spec.alpha$valid_QMARK_.invokeStatic(alpha.clj:776) at clojure.spec.alpha$valid_QMARK_.invoke(alpha.clj:772) at protojure.test.grpc$new_ShouldThrowResponse.invokeStatic(grpc.cljc:472) at protojure.test.grpc$new_ShouldThrowResponse.invoke(grpc.cljc:467) at protojure.grpc.codec.lpm$encode$fn__24309$fn__24406$state_machine__10008__auto____24433$fn__24435.invoke(lpm.clj:245) at protojure.grpc.codec.lpm$encode$fn__24309$fn__24406$state_machine__10008__auto____24433.invoke(lpm.clj:245) at clojure.core.async.impl.ioc_macros$run_state_machine.invokeStatic(ioc_macros.clj:978) at clojure.core.async.impl.ioc_macros$run_state_machine.invoke(ioc_macros.clj:977) at clojure.core.async.impl.ioc_macros$run_state_machine_wrapped.invokeStatic(ioc_macros.clj:982) at clojure.core.async.impl.ioc_macros$run_state_machine_wrapped.invoke(ioc_macros.clj:980) at clojure.core.async.impl.ioc_macros$take_BANG_$fn__10026.invoke(ioc_macros.clj:991) at clojure.core.async.impl.channels.ManyToManyChannel$fn__4751$fn__4752.invoke(channels.clj:99) at clojure.lang.AFn.run(AFn.java:22) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) at clojure.core.async.impl.concurrent$counted_thread_factory$reify__4620$fn__4621.invoke(concurrent.clj:29) at clojure.lang.AFn.run(AFn.java:22) at java.base/java.lang.Thread.run(Thread.java:833)