got: Throw helpful error messages

got.HTTPError has a lot of useful properties within request and response, but they are non-enumerable for some reason. This means that they don’t get printed by console.error() (which uses util.inspect() under the hood). The only property that is enumerable is timings, which is probably the least useful property out of all of them. This makes the log output of Got errors very verbose, with providing almost no useful information in that output (very bad signal/noise ratio):

HTTPError: Response code 403 (Forbidden)
    at Request.<anonymous> (node_modules/got/dist/source/as-promise/index.js:117:42)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  code: undefined,
  timings: {
    start: 1613304139851,
    socket: 1613304139851,
    lookup: 1613304139851,
    connect: 1613304139917,
    secureConnect: 1613304139986,
    upload: 1613304139986,
    response: 1613304140680,
    end: 1613304140681,
    error: undefined,
    abort: undefined,
    phases: {
      wait: 0,
      dns: 0,
      tcp: 66,
      tls: 69,
      request: 0,
      firstByte: 694,
      download: 1,
      total: 830
    }
  }
}

Agreed

TL;DR The default behavior of got library should balance security and pragmatism and should be configurable by the user

@szmarczak @sindresorhus I’m very happy to hear that error.options will become enumerable out of the box in got@12. I reported this a very long time ago here https://github.com/sindresorhus/got/issues/1067 I’m curious what changed in the meantime.

I’m afraid that making whole error.options enumerable it’s going to be too verbose unless irrelevant/null/undefined properties are filtered out but it’s definitely better than nothing.

Regarding error.body in most cases servers responding with 4xx or 5xx do not include sensitive data but information about why the error occurred. For example Google Storage API returns information about missing permissions which is very helpful for debugging (service account myapp does not have permission storage.objects.write for bucket mybucket).

On one side there is a security concern if

• a program blindly forwards and error to a client: this is no different than forwarding SQL errors to clients and should not be got’s concern
• a program blindly logs or prints error objects: it’s a reasonable issue and got should have sane defaults that allow for configurable behavior (beforeError handles probably all cases)

On the other side debugging every HTTP issue is taking minutes or hours instead of seconds OR you end up with a lot of boilerplate code OR you have to use a beforeError hook. None of those is preferable.

I propose that at the very least a set of properties considered safe are enumerable by default:

• method
• HTTP status code
• initial URL and final URL because URLs should never contain sensitive information given that it’s logged by servers and if it contains sensitive information is not got’s concern
• all headers keys but with some header values such as Authorization header replaced with ‘** sensitive information removed **’

Currently I end up with code like this because got errors are useless in an application that calls multiple endpoints. I can’t even know which API call failed unless I use a beforeError hook or this:

  async setMediaStatus(mediaId, status, message) {
      const url = `/media/${mediaId}/events`;
      const body = {
        key: 'status',
        meta: {
          type: status,
          message,
        },
      };

      try {
        await this.api.post(url, {
          json: body,
        }).json();
      } catch (err) {
        err.method = 'POST';
        err.url = url;
        err.body = body;
        throw err;
      }
  }

I would like to see a sane default behavior in a future release of got. I would prefer having sensitive information in logs over writing code like this or spending time trying to reproduce or guess an error to see which API call failed every time it happens.

In Got v12, error.options is enumerable so it looks like this:

HTTPError: Response code 404 (NOT FOUND)
    at Request.<anonymous> (file:///home/szm/Desktop/got/dist/source/as-promise/index.js:89:42)
    at Object.onceWrapper (node:events:485:26)
    at Request.emit (node:events:390:22)
    at Request.EventEmitter.emit (node:domain:532:15)
    at Request._onResponseBase (file:///home/szm/Desktop/got/dist/source/core/index.js:716:22)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
    at async Request._onResponse (file:///home/szm/Desktop/got/dist/source/core/index.js:755:13) {
  input: undefined,
  code: 'ERR_NON_2XX_3XX_RESPONSE',
  timings: {
    start: 1626356829477,
    socket: 1626356829477,
    lookup: 1626356829494,
    connect: 1626356829627,
    secureConnect: 1626356829763,
    upload: 1626356829764,
    response: 1626356829902,
    end: 1626356829904,
    error: undefined,
    abort: undefined,
    phases: {
      wait: 0,
      dns: 17,
      tcp: 133,
      tls: 136,
      request: 1,
      firstByte: 138,
      download: 2,
      total: 427
    }
  },
  options: {
    request: undefined,
    agent: { http: undefined, https: undefined, http2: undefined },
    h2session: undefined,
    decompress: true,
    timeout: {
      connect: undefined,
      lookup: undefined,
      read: undefined,
      request: undefined,
      response: undefined,
      secureConnect: undefined,
      send: undefined,
      socket: undefined
    },
    prefixUrl: '',
    body: undefined,
    form: undefined,
    json: undefined,
    cookieJar: undefined,
    ignoreInvalidCookies: false,
    searchParams: undefined,
    dnsLookup: undefined,
    dnsCache: undefined,
    context: {},
    hooks: {
      init: [],
      beforeRequest: [],
      beforeError: [],
      beforeRedirect: [],
      beforeRetry: [],
      afterResponse: []
    },
    followRedirect: true,
    maxRedirects: 10,
    cache: undefined,
    throwHttpErrors: true,
    username: '',
    password: '',
    http2: false,
    allowGetBody: false,
    headers: {
      'user-agent': 'got (https://github.com/sindresorhus/got)',
      accept: 'application/json',
      'accept-encoding': 'gzip, deflate, br'
    },
    methodRewriting: false,
    dnsLookupIpVersion: undefined,
    parseJson: [Function: parse],
    stringifyJson: [Function: stringify],
    retry: {
      limit: 2,
      methods: [ 'GET', 'PUT', 'HEAD', 'DELETE', 'OPTIONS', 'TRACE' ],
      statusCodes: [
        408, 413, 429, 500,
        502, 503, 504, 521,
        522, 524
      ],
      errorCodes: [
        'ETIMEDOUT',
        'ECONNRESET',
        'EADDRINUSE',
        'ECONNREFUSED',
        'EPIPE',
        'ENOTFOUND',
        'ENETUNREACH',
        'EAI_AGAIN'
      ],
      maxRetryAfter: undefined,
      calculateDelay: [Function: calculateDelay],
      backoffLimit: Infinity,
      noise: 100
    },
    localAddress: undefined,
    method: 'GET',
    createConnection: undefined,
    cacheOptions: {
      shared: undefined,
      cacheHeuristic: undefined,
      immutableMinTimeToLive: undefined,
      ignoreCargoCult: undefined
    },
    httpsOptions: {
      alpnProtocols: undefined,
      rejectUnauthorized: undefined,
      checkServerIdentity: undefined,
      certificateAuthority: undefined,
      key: undefined,
      certificate: undefined,
      passphrase: undefined,
      pfx: undefined,
      ciphers: undefined,
      honorCipherOrder: undefined,
      minVersion: undefined,
      maxVersion: undefined,
      signatureAlgorithms: undefined,
      tlsSessionLifetime: undefined,
      dhparam: undefined,
      ecdhCurve: undefined,
      certificateRevocationLists: undefined
    },
    encoding: undefined,
    resolveBodyOnly: false,
    isStream: false,
    responseType: 'text',
    url: URL {
      href: 'https://httpbin.org/https://httpbin.org/status/404',
      origin: 'https://httpbin.org',
      protocol: 'https:',
      username: '',
      password: '',
      host: 'httpbin.org',
      hostname: 'httpbin.org',
      port: '',
      pathname: '/https://httpbin.org/status/404',
      search: '',
      searchParams: URLSearchParams {},
      hash: ''
    },
    pagination: {
      transform: [Function: transform],
      paginate: [Function: paginate],
      filter: [Function: filter],
      shouldContinue: [Function: shouldContinue],
      countLimit: Infinity,
      backoff: 0,
      requestLimit: 10000,
      stackAllItems: false
    },
    setHost: true,
    maxHeaderSize: undefined,
    searchParameters: undefined
  }
}

We could attach error.body but trimmed down to 100 characters.

@sindresorhus Any thoughts?

I agree too. I’m starting to use the lib and that was my first unconfortable usecase. Would be awesome to receive formated erros by default.

I made this to change Got Error and get response error (I use got-cjs, not directly got) :

import type { OptionsOfUnknownResponseBody, RequestError } from 'got-cjs';
import { got as originGot } from 'got-cjs';

interface GotErrorResponse {
    statusCode: number,
    body: unknown,
}

export class GotError extends Error {
    response: GotErrorResponse;

    constructor(err: RequestError) {
        const statusCode        = err.response?.statusCode || 0;
        const { method, url }   = err.options;
        const body              = err.response?.body || err.message;
        const message           = [[statusCode, method, url]
            .filter(Boolean)
            .join(' '), JSON.stringify(body)]
            .filter(Boolean)
            .join('\n');


        super(message);

        this.response = {
            statusCode,
            body,
        };
    }
}

/**
 * Wrapper to got library
 *
 * @param uri - URI to request
 * @param options - Got options
 * @returns Request's response
 */
export const got = async (uri: string | URL, options?: OptionsOfUnknownResponseBody) => {
    try {
        return await originGot(uri, options);
    } catch (err) {
        throw new GotError(err);
    }
};

For the record, I (the topic starter) “solved” it for myself by wrapping got with some function that adds beforeError hook by default (and alters some other defaults, unrelated to this issue), and it lives here: https://github.com/NaturalCycles/nodejs-lib/blob/master/src/got/getGot.ts#L68

@aalexgabi, your last message very well described the experience that I had when opening this issue.

No one has commented whether it’s safe to expose the body on the error though. I’m not against doing it, but I am concerned it will later turn into a security vulnerability of some kind. Someone will have to do some research about it.

One way to figure this out would be to look at other popular request libraries. For example, the Python one doesn’t seem to expose the body directly on the error: https://stackoverflow.com/a/58085336/64949

There are multiple ways we could solve this:

• Only do it when process.env.NODE_ENV === 'DEBUG'.
• Make it opt-in via an option.
• Encourage using diagnostic channel for debugging: https://github.com/sindresorhus/got/issues/1792

The problem is we can’t identify what kind of actual errors are really sent from the API, for example, 400 Bad Request could mean an invalid token but it could be mean something else too.