feat: properly decode error in fallback scenario, export FallbackServiceError type#866
feat: properly decode error in fallback scenario, export FallbackServiceError type#866alexander-fenster merged 3 commits intomasterfrom
Conversation
03e1118 to
a9d0017
Compare
Codecov Report
@@ Coverage Diff @@
## master googleapis/gax-nodejs#866 +/- ##
==========================================
- Coverage 88.72% 88.62% -0.10%
==========================================
Files 40 40
Lines 6413 6429 +16
Branches 558 557 -1
==========================================
+ Hits 5690 5698 +8
- Misses 720 728 +8
Partials 3 3
Continue to review full report at Codecov.
|
src/fallbackError.ts
Outdated
| // Adapted from https://github.com/grpc/grpc-node/blob/master/packages/grpc-js/src/call.ts#L79 | ||
| callErrorFromStatus(status: DecodedRpcStatus): Error { | ||
| const message = `${status.code} ${Status[status.code]}: ${status.message}`; | ||
| delete status.message; |
There was a problem hiding this comment.
Why delete the message from the object?
There was a problem hiding this comment.
In line 85, when I do new Error(message) it sets the message, but Object.assign will overwrite it from status.message. I can either delete it, or assign status.message = message (I can change it to the latter if you think it's more readable).
There was a problem hiding this comment.
Changed this to more readable code.
There was a problem hiding this comment.
Nope that's fine! Just poking.
src/fallbackError.ts
Outdated
| callErrorFromStatus(status: DecodedRpcStatus): Error { | ||
| const message = `${status.code} ${Status[status.code]}: ${status.message}`; | ||
| delete status.message; | ||
| return Object.assign(new Error(message), status); |
There was a problem hiding this comment.
This line of code is confusing to me. It looks like you're smashing a DecodedRpcStatus onto an Error object, but not changing the type. Should we return a type here that reflects the properties you're returning?
There was a problem hiding this comment.
Yeah, it's ServiceError from gRPC, I will import and use it here.
There was a problem hiding this comment.
Well no, I don't have any Metadata so I cannot use the type from gRPC but I'm now exporting my own (similar) type.
|
This is ready for one more look @JustinBeckwith! |
src/fallbackError.ts
Outdated
| // Adapted from https://github.com/grpc/grpc-node/blob/master/packages/grpc-js/src/call.ts#L79 | ||
| callErrorFromStatus(status: DecodedRpcStatus): Error { | ||
| const message = `${status.code} ${Status[status.code]}: ${status.message}`; | ||
| delete status.message; |
There was a problem hiding this comment.
Nope that's fine! Just poking.
Fixes googleapis/google-cloud-node-core#395. Adapted a piece of error processing from
@grpc/grpc-jsto use in the fallback scenario.