Fix Error handling and add test coverage for Thrift2ProtoAdapter

[natemort]

What changed?

• Remove request logging from GrpcStubs. This was a debugging tool added during implementation and isn't generally useful.
• Fix GRPC error handling for the handful of Async methods. These were previously being completed with an ExecutionException wrapping the StatusRuntimException rather than the StatusRuntimeException being mapped to the appropriate type.
• Increase test coverage

Why?

How did you test it?

Potential risks

Release notes

Documentation Changes

[codecov]

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.57%. Comparing base (6d76d41) to head (8e2b988).
Report is 11 commits behind head on master.

Files with missing lines	Patch %	Lines
...ce/internal/compatibility/Thrift2ProtoAdapter.java	76.92%	1 Missing and 2 partials ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
...ce/internal/compatibility/Thrift2ProtoAdapter.java	93.14% <76.92%> (+93.14%)	⬆️

... and 19 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5d8ef8...8e2b988. Read the comment docs.

[coveralls]

Pull Request Test Coverage Report for Build 2630

Details

• 11 of 13 (84.62%) changed or added relevant lines in 1 file are covered.
• 2 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+3.2%) to 82.827%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/main/java/com/uber/cadence/internal/compatibility/Thrift2ProtoAdapter.java	11	13	84.62%

Files with Coverage Reduction	New Missed Lines	%
src/main/java/com/uber/cadence/internal/testservice/TestWorkflowMutableStateImpl.java	1	85.33%
src/main/java/com/uber/cadence/internal/common/InternalUtils.java	1	79.61%

Totals
Change from base Build 2628:	3.2%
Covered Lines:	16042
Relevant Lines:	19368

---

💛 - Coveralls

[shijiesheng]

I would probably just keep it. For unit test, we can just cover the lines without asserting as it's for debugging purposes.

[shijiesheng]

Also seeing this pattern in multiple other entities. For example ActivityPollTask

[natemort]

Sounds good, readded with test coverage.

[davidporter-id-au]

I'm not 100% sure how much we care about these, but if we do care about them, is there some kind of assertion we can add to ensure that they're mapped for all future fields too? (maybe reflection?)

[natemort]

For any type that we have a mapper for we use MapperTestUtil to assert that all fields are set (or a select set are missing if we haven't updated the mappers for them). We do similar things with some of the types contained within the requests(decisions and history events) as well as enums.

These types don't actually exist in thrift (the thrift APIs return void, something we never do in gRPC) so there aren't any mappers. It seems very unlikely we'd add fields to these types until after we completely remove thrift, at which point this Thrift2Proto layer won't exist at all. So I don't think it's worth testing these ones the way all the others are tested.