Skip to content

Commit

Permalink
Treat response.text() as a promise in error handling. (#11)
Browse files Browse the repository at this point in the history
We [recently added some better exception handling](#9) but unfortunately printing the response body isn't working because [`response.text()` is a Promise](https://github.github.io/fetch/#response-body). The message looks like this: `ThriftRequestError: Thrift request failed: HTTP 503 Service Unavailable: [object Promise]`

The fix is to use `.then()` to get the body.

I have not tested this. I did run `npm run test` and some of the tests pass, but 75 out of 108 thrift-integration tests fail. I'm seeing this error a lot:
```
request to http://localhost:8090/thrift failed, reason: connect ECONNREFUSED ::1:8090
```
I think it also might have been happening before this change. It looks unrelated. It seems like it's happening because [Hapi listens on 127.0.0.1](https://hapi.dev/api/?v=20.2.2#-serveroptionsaddress) and [Node v17 and higher stopped forcing IPv64addresses to be listed first when resolving domains](nodejs/node#40537 (comment)) so now IPv6 addresses can be returned first. I'll probably post a separate PR for that, but for now I'd prefer not to block this change.
  • Loading branch information
markdoliner-doma committed Jul 6, 2022
1 parent 9c02a7d commit b4a4751
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions packages/thrift-client/src/main/connections/HttpConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,20 @@ export class HttpConnection extends ThriftConnection<RequestOptions> {
if (shouldRetry(response, retry, this.withEndpointPerMethod)) {
resolve(this.write(dataToWrite, methodName, options, true))
} else if (isErrorResponse(response)) {
reject(
new ThriftRequestError(
`Thrift request failed: HTTP ${response.status} ${
response.statusText
}: ${response.text()}`,
response
const baseMessage = `Thrift request failed: HTTP ${response.status} ${response.statusText}`
response
.text()
.then(responseText =>
reject(new ThriftRequestError(`${baseMessage}: ${responseText}`, response))
)
)
.catch(reason => {
reject(
new ThriftRequestError(
`${baseMessage} and the response body could not be read: ${reason}`,
response
)
)
})
} else {
response.arrayBuffer().then(arrayBuffer =>
resolve({
Expand Down

0 comments on commit b4a4751

Please sign in to comment.