Client should use a subclass of Error when rejecting upon HTTP error response

[markdoliner-doma]

When making a thrift request with HttpConnection, if the server returns a non-2xx response code currently the full response object is included in the promise rejection. According to the DefinitelyTyped definitions it's an object that looks like an http.IncomingMessage.

I think it would be better if HttpConnection abstracted away this detail and threw some sort of Error object, instead. There's certainly no rule saying promise rejections must be an instance of Error, but it does seem like a polite thing to do. MDN recommends it ("For debugging purposes and selective error catching, it is useful to make reason an instanceof Error."). This only-mildly-populate Stack Overflow response agrees. I think the point about "it's useful to have a stack trace" is a good one. These days people like using await with promises, in which case the rejection object is thrown, and who knows where that's going to get caught and logged. Having a stack trace is valuable in this situation.

A caveat is that it looks like it's possible to include a requestImpl key in the options object passed to createHttpClient and that will get passed on to HttpConnection and that allows developers to specify an alternative to request. I don't know if that complicates things. If someone is specifying their own request library, maybe they would want to receive the full response object in the rejection? Or maybe it would be difficult for HttpConnection to extract meaningful info from the response? But I don't know, HttpConnection is already assuming the response as statusCode and body fields, maybe that's good enough.

I'm repro'ing this with a script along these lines:

import { createHttpClient } from '@statestitle/thrift-client'
import { MyThriftApi } from 'my_thrift_api'

export async function run(): Promise<void> {
  const client = createHttpClient(MyThriftApi.Client, {
    hostName: 'getstatuscode.com',
    port: 80,
    path: '/503',
  })

  try {
    await client.getExampleWidgets()
  } catch (e) {
    console.log('Caught exception', e)
  }
}

run()

It logs:

Caught exception IncomingMessage {
  _readableState: ReadableState {
    objectMode: false,
    highWaterMark: 16384,
    ...etc
  },
    ... a ton of other objects, functions, etc.
}