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

[DRAFT] Fix RPC handler when lazy response data throws an error #129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sundbry
Copy link
Contributor

@sundbry sundbry commented Mar 19, 2022

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 (mapv instead of map etc). 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.clj and 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)

sundbry added 2 commits March 19, 2022 01:11
The `protos` target should generate code under test/test/ instead of just
test/

Signed-off-by: Ryan Sundberg <[email protected]>
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 <[email protected]>
@ghaskins
Copy link
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.

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

Successfully merging this pull request may close these issues.

2 participants