Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Client won't throw exception on void function #17

Merged
merged 2 commits into from

5 participants

Luka Zakrajšek Chris Aniszczyk Lee Packham Sebastien Rainville Chunyan Song
Luka Zakrajšek

If you have void function in your service that throws an exception, client will always silently result with unit (Future.Done).

Chris Aniszczyk
Owner

Any thoughts on this @chunyan ?

Lee Packham

Any chance this can move forwards? This causes problems for things like cassandra.thrift, for example.

Sebastien Rainville

+1 on moving this forward

Chunyan Song

My apologies. Scrooge had been short staffed, and we didn't stay on top of all things on Github. I will go through all pull requests and issues this week.

Chunyan Song chunyan commented on the diff
...om/twitter/scrooge/backend/ServiceGeneratorSpec.scala
@@ -239,6 +239,17 @@ class ServiceGeneratorSpec extends SpecificationWithJUnit with EvalHelper with J
client.deliver("Boston")() mustEqual 42
}
+ "success void" in {
+ val request = encodeRequest("remove", ExceptionalService.RemoveArgs(123))
Chunyan Song
chunyan added a note

We have changed the naming of internal marshalling functions for args and result. So please use remove$args and remove$result instead of RemoveArgs and RemoveResult.

Everything else looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Chunyan Song chunyan closed this
Chunyan Song chunyan reopened this
Chunyan Song chunyan merged commit 4855f16 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
2  scrooge-generator/src/main/resources/scalagen/finagleClientFunction.scala
View
@@ -17,7 +17,7 @@ private[this] object {{__stats_name}} {
None
{{/hasThrows}}
{{#void}}
- Future.Done
+ exception.getOrElse(Future.Done)
{{/void}}
{{^void}}
exception.orElse(result.success.map(Future.value)).getOrElse(Future.exception(missingResult("{{clientFuncName}}")))
23 scrooge-generator/src/test/scala/com/twitter/scrooge/backend/ServiceGeneratorSpec.scala
View
@@ -239,6 +239,17 @@ class ServiceGeneratorSpec extends SpecificationWithJUnit with EvalHelper with J
client.deliver("Boston")() mustEqual 42
}
+ "success void" in {
+ val request = encodeRequest("remove", ExceptionalService.RemoveArgs(123))
Chunyan Song
chunyan added a note

We have changed the naming of internal marshalling functions for args and result. So please use remove$args and remove$result instead of RemoveArgs and RemoveResult.

Everything else looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ val response = encodeResponse("remove", ExceptionalService.RemoveResult())
+
+ expect {
+ one(impl).remove(123) willReturn Future.Done
+ }
+
+ client.remove(123)() mustEqual ()
+ }
+
"exception" in {
val request = encodeRequest("deliver", ExceptionalService.DeliverArgs("Boston"))
val ex = Xception(1, "boom")
@@ -250,6 +261,18 @@ class ServiceGeneratorSpec extends SpecificationWithJUnit with EvalHelper with J
client.deliver("Boston")() must throwA[ThriftException](ex)
}
+
+ "void exception" in {
+ val request = encodeRequest("remove", ExceptionalService.RemoveArgs(123))
+ val ex = Xception(1, "boom")
+ val response = encodeResponse("remove", ExceptionalService.RemoveResult(ex = Some(ex)))
+
+ expect {
+ one(impl).remove(123) willReturn Future.exception(ex)
+ }
+
+ client.remove(123)() must throwA[ThriftException](ex)
+ }
}
"correctly inherit traits across services" in {
6 scrooge-generator/src/test/thrift/test.thrift
View
@@ -305,6 +305,12 @@ service ExceptionalService {
2: Xception2 ex2
3: EmptyXception ex3
)
+
+ void remove(1: i32 id) throws (
+ 1: Xception ex
+ 2: Xception2 ex2
+ 3: EmptyXception ex3
+ )
}
service ThriftTest
Something went wrong with that request. Please try again.