Skip to content

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

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.

Sign up for GitHub

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

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db110e4
Makefile: Fix `protos` target and regenerate code
sundbry Mar 18, 2022
f4a74d9
New test case: RPC handler when lazy response data throws an error
sundbry Mar 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ clean:

.PHONY: protos
protos:
protoc --clojure_out=grpc-client,grpc-server:test --proto_path=test/resources $(shell find test/resources -name "*.proto" | sed 's|test/resources/||g')
mkdir -p test/test/resources
protoc --clojure_out=grpc-client,grpc-server:test/test --proto_path=test/test/resources $(shell find test/test/resources -name "*.proto" | sed 's|test/test/resources/||g')
63 changes: 33 additions & 30 deletions test/test/com/example/addressbook.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@
;-----------------------------------------------------------------------------
(def Person-PhoneType-default :mobile)

(def Person-PhoneType-val2label {0 :mobile
1 :home
2 :work})
(def Person-PhoneType-val2label {
0 :mobile
1 :home
2 :work})

(def Person-PhoneType-label2val (set/map-invert Person-PhoneType-val2label))

Expand All @@ -58,6 +59,8 @@
([tag options value os]
(serdes.core/write-Enum tag options (get-Person-PhoneType value) os)))



;;----------------------------------------------------------------------------------
;;----------------------------------------------------------------------------------
;; Message Implementations
Expand All @@ -82,23 +85,23 @@
(s/def :com.example.addressbook.Person/id int?)
(s/def :com.example.addressbook.Person/email string?)

(s/def ::Person-spec (s/keys :opt-un [:com.example.addressbook.Person/name :com.example.addressbook.Person/id :com.example.addressbook.Person/email]))
(def Person-defaults {:name "" :id 0 :email "" :phones []})
(s/def ::Person-spec (s/keys :opt-un [:com.example.addressbook.Person/name :com.example.addressbook.Person/id :com.example.addressbook.Person/email ]))
(def Person-defaults {:name "" :id 0 :email "" :phones [] })

(defn cis->Person
"CodedInputStream to Person"
[is]
(->> (tag-map Person-defaults
(fn [tag index]
(case index
1 [:name (serdes.core/cis->String is)]
2 [:id (serdes.core/cis->Int32 is)]
3 [:email (serdes.core/cis->String is)]
4 [:phones (serdes.complex/cis->repeated ecis->Person-PhoneNumber is)]
(fn [tag index]
(case index
1 [:name (serdes.core/cis->String is)]
2 [:id (serdes.core/cis->Int32 is)]
3 [:email (serdes.core/cis->String is)]
4 [:phones (serdes.complex/cis->repeated ecis->Person-PhoneNumber is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->Person-record)))
[index (serdes.core/cis->undefined tag is)]))
is)
(map->Person-record)))

(defn ecis->Person
"Embedded CodedInputStream to Person"
Expand Down Expand Up @@ -136,21 +139,21 @@

(s/def :com.example.addressbook.Person-PhoneNumber/number string?)
(s/def :com.example.addressbook.Person-PhoneNumber/type (s/or :keyword keyword? :int int?))
(s/def ::Person-PhoneNumber-spec (s/keys :opt-un [:com.example.addressbook.Person-PhoneNumber/number :com.example.addressbook.Person-PhoneNumber/type]))
(def Person-PhoneNumber-defaults {:number "" :type Person-PhoneType-default})
(s/def ::Person-PhoneNumber-spec (s/keys :opt-un [:com.example.addressbook.Person-PhoneNumber/number :com.example.addressbook.Person-PhoneNumber/type ]))
(def Person-PhoneNumber-defaults {:number "" :type Person-PhoneType-default })

(defn cis->Person-PhoneNumber
"CodedInputStream to Person-PhoneNumber"
[is]
(->> (tag-map Person-PhoneNumber-defaults
(fn [tag index]
(case index
1 [:number (serdes.core/cis->String is)]
2 [:type (cis->Person-PhoneType is)]
(fn [tag index]
(case index
1 [:number (serdes.core/cis->String is)]
2 [:type (cis->Person-PhoneType is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->Person-PhoneNumber-record)))
[index (serdes.core/cis->undefined tag is)]))
is)
(map->Person-PhoneNumber-record)))

(defn ecis->Person-PhoneNumber
"Embedded CodedInputStream to Person-PhoneNumber"
Expand Down Expand Up @@ -185,19 +188,19 @@
"com.example.addressbook.AddressBook"))

(s/def ::AddressBook-spec (s/keys :opt-un []))
(def AddressBook-defaults {:people []})
(def AddressBook-defaults {:people [] })

(defn cis->AddressBook
"CodedInputStream to AddressBook"
[is]
(->> (tag-map AddressBook-defaults
(fn [tag index]
(case index
1 [:people (serdes.complex/cis->repeated ecis->Person is)]
(fn [tag index]
(case index
1 [:people (serdes.complex/cis->repeated ecis->Person is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->AddressBook-record)))
[index (serdes.core/cis->undefined tag is)]))
is)
(map->AddressBook-record)))

(defn ecis->AddressBook
"Embedded CodedInputStream to AddressBook"
Expand Down
75 changes: 39 additions & 36 deletions test/test/com/example/empty.cljc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,20 @@

(defn convert-Selection-opt [origkeyval]
(cond
(get-in origkeyval [:opt :e]) (update-in origkeyval [:opt :e] new-Empty)
(get-in origkeyval [:opt :ne]) (update-in origkeyval [:opt :ne] new-NonEmpty)
:default origkeyval))
(get-in origkeyval [:opt :e]) (update-in origkeyval [:opt :e] new-Empty)
(get-in origkeyval [:opt :ne]) (update-in origkeyval [:opt :ne] new-NonEmpty)
:default origkeyval))

(defn write-Selection-opt [opt os]
(let [field (first opt)
k (when-not (nil? field) (key field))
v (when-not (nil? field) (val field))]
(case k
:e (serdes.core/write-embedded 1 v os)
:ne (serdes.core/write-embedded 2 v os)
nil)))
(case k
:e (serdes.core/write-embedded 1 v os)
:ne (serdes.core/write-embedded 2 v os)
nil)))



;;----------------------------------------------------------------------------------
;;----------------------------------------------------------------------------------
Expand All @@ -63,7 +65,8 @@
;-----------------------------------------------------------------------------
(defrecord Empty-record []
pb/Writer
(serialize [this os])
(serialize [this os]
)
pb/TypeReflection
(gettype [this]
"com.example.empty.Empty"))
Expand All @@ -75,11 +78,11 @@
"CodedInputStream to Empty"
[is]
(->> (tag-map Empty-defaults
(fn [tag index]
(case index
[index (serdes.core/cis->undefined tag is)]))
is)
(map->Empty-record)))
(fn [tag index]
(case index
[index (serdes.core/cis->undefined tag is)]))
is)
(map->Empty-record)))

(defn ecis->Empty
"Embedded CodedInputStream to Empty"
Expand Down Expand Up @@ -114,20 +117,20 @@
"com.example.empty.NonEmpty"))

(s/def :com.example.empty.NonEmpty/i int?)
(s/def ::NonEmpty-spec (s/keys :opt-un [:com.example.empty.NonEmpty/i]))
(def NonEmpty-defaults {:i 0})
(s/def ::NonEmpty-spec (s/keys :opt-un [:com.example.empty.NonEmpty/i ]))
(def NonEmpty-defaults {:i 0 })

(defn cis->NonEmpty
"CodedInputStream to NonEmpty"
[is]
(->> (tag-map NonEmpty-defaults
(fn [tag index]
(case index
1 [:i (serdes.core/cis->Int32 is)]
(fn [tag index]
(case index
1 [:i (serdes.core/cis->Int32 is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->NonEmpty-record)))
[index (serdes.core/cis->undefined tag is)]))
is)
(map->NonEmpty-record)))

(defn ecis->NonEmpty
"Embedded CodedInputStream to NonEmpty"
Expand Down Expand Up @@ -168,14 +171,14 @@
"CodedInputStream to Selection"
[is]
(->> (tag-map Selection-defaults
(fn [tag index]
(case index
1 [:opt {:e (ecis->Empty is)}]
2 [:opt {:ne (ecis->NonEmpty is)}]
(fn [tag index]
(case index
1 [:opt {:e (ecis->Empty is)}]
2 [:opt {:ne (ecis->NonEmpty is)}]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->Selection-record)))
[index (serdes.core/cis->undefined tag is)]))
is)
(map->Selection-record)))

(defn ecis->Selection
"Embedded CodedInputStream to Selection"
Expand Down Expand Up @@ -218,14 +221,14 @@
"CodedInputStream to Container"
[is]
(->> (tag-map Container-defaults
(fn [tag index]
(case index
1 [:e (ecis->Empty is)]
2 [:ne (ecis->NonEmpty is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->Container-record)))
(fn [tag index]
(case index
1 [:e (ecis->Empty is)]
2 [:ne (ecis->NonEmpty is)]

[index (serdes.core/cis->undefined tag is)]))
is)
(map->Container-record)))

(defn ecis->Container
"Embedded CodedInputStream to Container"
Expand Down
44 changes: 34 additions & 10 deletions test/test/protojure/grpc_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,20 @@
{:body {:msg (str "Hello, " auth)}}))

(ShouldThrow
[_ request]
(throw (ex-info "This is supposed to fail" {})))
[_ {{test-case :case} :grpc-params}]
{:body
(case test-case
0 (throw (ex-info "This is supposed to fail" {}))
; Non-lazy seq
1 {:numbers
(mapv (fn [_]
(throw (ex-info "This is also supposed to fail (1)" {})))
(range 3))}
; Lazy seq
2 {:numbers
(map (fn [n]
(throw (ex-info "This is also supposed to fail (2)" {})))
(range 3))})})

(Async
[_ request]
Expand Down Expand Up @@ -262,6 +274,16 @@
[code & body]
`(-check-throw ~code #(do ~@body)))

(defn is-grpc-error? [grpc-status response-promise]
"Asserts that response promise returns a specific gRPC error code, and
returns the exception."
(let [ex (is (thrown? Exception @response-promise))
cause (.getCause ex)
code (grpc.status/get-code grpc-status)]
(is (some? cause))
(is (= code (:status (ex-data cause))))
cause))

;;-----------------------------------------------------------------------------
;; Scaletest Assemblies
;;-----------------------------------------------------------------------------
Expand Down Expand Up @@ -680,18 +702,20 @@
(deftest test-grpc-exception
(let [client @(grpc.http2/connect {:uri (str "http://localhost:" (:port @test-env))})]
(testing "Check that exceptions thrown on the server propagate back to the client"
(is (thrown? java.util.concurrent.ExecutionException
@(test.client/ShouldThrow client {})))
(try
@(test.client/ShouldThrow client {})
(assert false) ;; we should never get here
(catch java.util.concurrent.ExecutionException e
(let [{:keys [status]} (ex-data (.getCause e))]
(is (= status 13))))))
(is-grpc-error? :internal (test.client/ShouldThrow client {})))
(testing "Check that we can still connect even after exceptions have been received"
(is (-> @(test.client/Async client {}) :msg (= "Hello, Async"))))
(grpc/disconnect client)))

(deftest test-grpc-sequence-exception
(let [client @(grpc.http2/connect {:uri (str "http://localhost:" (:port @test-env))})]
(testing "Check that exceptions thrown in sequence mappers on the server propagate back to the client"
(testing "Non-lazy: throws in mapv"
(is-grpc-error? :internal (test.client/ShouldThrow client {:case 1})))
(testing "Lazy: throws in map"
(is-grpc-error? :internal (test.client/ShouldThrow client {:case 2}))))
(grpc/disconnect client)))

(deftest test-grpc-async
(testing "Check that async processing functions correctly"
(let [client @(grpc.http2/connect {:uri (str "http://localhost:" (:port @test-env))})]
Expand Down
Loading