Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

fix: Fix handling of null responses in rest transport #1668

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

vam-google
Copy link
Contributor

Null response should not ever happen, but apparently it is possible in cases when errors occur somewhere on network/authentication level

Null response should not ever happen, but apparently it is possible in cases when errors occur somewhere on network/authentication level
@vam-google vam-google requested review from a team as code owners April 28, 2022 05:39
@vam-google vam-google changed the title fix: Fix handling of null response handling in rest transport fix: Fix handling of null responses in rest transport Apr 28, 2022
statusCode,
"Exception during a client call closure",
new NullPointerException(
"Both response message and response exception were null")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this message should be "Either response message or response exception is null"?

Copy link
Contributor Author

@vam-google vam-google Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should mean that both of them are null (i.e. and not or). One of them being null is Ok. Either one being null has been handled before. Specifically !future.isDone() means that there was no response message (null), trailers.getException() == null means that there was no excepiton either. I did not even think it was poissible combination, but apparently it is possible for a super-broken responses from server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, so both check trailers == null and trailers.getException() == null are checking if the response exception exist or not. Yeah I found similar things when I was implementing the Error Details feature, where I checked both trailer and trailer error.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.9% 90.9% Coverage
0.0% 0.0% Duplication

@vam-google vam-google merged commit 8def947 into googleapis:main Apr 29, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request May 10, 2022
🤖 I have created a release *beep* *boop*
---


## [2.17.0](v2.16.0...v2.17.0) (2022-05-10)


### Features

* next release from main branch is 2.17.0 ([#1659](#1659)) ([5a31361](5a31361))


### Bug Fixes

* Fix handling of null responses in rest transport ([#1668](#1668)) ([8def947](8def947))
* use graal-sdk 21.3.2 ([#1670](#1670)) ([92c2697](92c2697))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
suztomo pushed a commit to googleapis/sdk-platform-java that referenced this pull request Dec 16, 2022
🤖 I have created a release *beep* *boop*
---


## [2.17.0](googleapis/gax-java@v2.16.0...v2.17.0) (2022-05-10)


### Features

* next release from main branch is 2.17.0 ([#1659](googleapis/gax-java#1659)) ([475b57b](googleapis/gax-java@475b57b))


### Bug Fixes

* Fix handling of null responses in rest transport ([#1668](googleapis/gax-java#1668)) ([49c6d70](googleapis/gax-java@49c6d70))
* use graal-sdk 21.3.2 ([#1670](googleapis/gax-java#1670)) ([ac55789](googleapis/gax-java@ac55789))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants