Endpoint returning Alias of optional<T> + Http 204 results in NeverReturnNullDecoder exception

[nartmal]

What happened?

An internal service has legacy, non-conjurized, endpoints that return null which breaks with the NeverNullDecoder being added as a default. We're pretty low in the stack so major releasing just to address this break is not a feasible option for us. This will prevent us from taking all future updates.

What did you want to happen?

Either remove it completely or make the NeverNullDecoder optional.

[iamdanfox]

@nartmal the initial implementation of this was added in #766 (released 3.40.0) as a defensive measure so that all users can be certain that they'll never have to deal with nulls (we always recommend using Optional instead of null). However, we quickly realized this had undesirable behaviour when the service should return a container type (List, Set, Optional, Map). This was fixed in #772 (released 3.41.0).

Which version were you experiencing problems with? I don't really want to remove it completely as it is beneficial to the majority of our users.

If you have defined your API to have nullable methods then it's not really sufficient to stop taking upgrades as I think any consumer that uses modern conjure-java-runtime will be unable to use your nullable endpoints. I'd recommend adding an interface to your API that returns optionals and deprecating your existing endpoints.

[nartmal]

This is closable, tliao & jbaker have a path to resolve without any action item here.

[iamdanfox]

@nartmal can you share the approach you're planning to take just in case other people find themselves in the same situation?

[tengfliao]

@iamdanfox The underlying issue was that there was a type Type that was an alias of Optional<String>. The alias wasn't being treated the same way as if it were an actual Optional<String> and the decoder interpreted it as a null. The approach we're planning on taking is not defining an alias and having java clients take the break.

[iamdanfox]

I've encountered this problem myself.

1. In the Conjure definition, we have an alias of an optional:

      OptionalAlias: { alias: optional<integer> }
...
      foo:
        http: GET /some-path
        returns: OptionalAlias

2. Then the server returns 204 (see below)

curl -v http://localhost:8000/some-path
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /some-path HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 204 No Content
< access-control-allow-origin: *
< content-length: 0
< date: Fri, 12 Oct 2018 14:21:31 GMT
< 
* Connection #0 to host localhost left intact

3. I get the following stacktrace:

java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.palantir.conjure.java.compliance.AutoDeserializeTest.expectSuccess(AutoDeserializeTest.java:98)
	at com.palantir.conjure.java.compliance.AutoDeserializeTest.runTestCase(AutoDeserializeTest.java:90)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: feign.codec.DecodeException: Unexpected null body
	at feign.SynchronousMethodHandler.decode(SynchronousMethodHandler.java:165)
	at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:129)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:76)
	at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:103)
	at com.sun.proxy.$Proxy15.receiveRawOptionalExample(Unknown Source)
	... 46 more
Caused by: com.palantir.logsafe.exceptions.SafeNullPointerException: Unexpected null body
	at com.palantir.logsafe.Preconditions.checkNotNull(Preconditions.java:215)
	at feign.NeverReturnNullDecoder.decode(NeverReturnNullDecoder.java:35)
	at feign.SynchronousMethodHandler.decode(SynchronousMethodHandler.java:161)
	... 50 more

What should have happened

My OkhttpClient should have somehow figured out that a HTTP 204 response should be translated into an instance of the OptionalAlias.of(OptionalInt.empty()).

I think this problem would also reproduce if the return type was an alias of list<T>, alias of set<T>, alias of map<K,V>.

[iamdanfox]

Fixed for feign in https://github.com/palantir/conjure-java-runtime/pull/865.

Still gotta fix this for retrofit!

[stale[bot]]

This issue has been automatically marked as stale because it has not been touched in the last 60 days. Please comment if you'd like to keep it open, otherwise it'll be closed in 7 days time.

[stale[bot]]

This issue has been automatically marked as stale because it has not been touched in the last 60 days. Please comment if you'd like to keep it open, otherwise it'll be closed in 7 days time.