Allow for http client to be swappable about node-openid-client

[panva]

Hey @JoshBarr, couldn't get https://github.com/sindresorhus/got#proxies to work?

I recognize the completeness of request, so rather than resolving to forks and duplicate packages I'll figure out a way to use other request libs.

[panva]

I think it shouldn't be that big of a deal as soon as we figure out a good interface. Your wrapper helps!

[panva]

https://github.com/panva/node-openid-client/tree/feature/swappable-request-client

Issuer.httpClient = {
   get(url, options) {}, // return Promise
   post(url, options) {}, // return Promise
   HTTPError, // used error constructor
};

@JoshBarr opinion? I tried with your got-like request and got all tests to pass/

[JoshBarr]

that looks awesome, I like very much!

[panva]

@JoshBarr ^1.11.0 has landed, you can now plug request instead of got. Should you be moving forward with the fork please remove the certification notes from the README.

[big8extreme]

thx josh, but can we have a proper exemple please?
this link not is not working
https://github.com/panva/node-openid-client/tree/feature/swappable-request-client

/opt/app-root/src/node_modules/got/index.js:229
--
  | req.connection.once('connect', () => {
  | ^
  | TypeError: Cannot read property 'once' of null
  | at EventEmitter.ee.once.req (/opt/app-root/src/node_modules/got/index.js:229:19)
  | at EventEmitter.g (events.js:291:16)
  | at emitOne (events.js:101:20)
  | at EventEmitter.emit (events.js:188:7)
  | at Immediate.setImmediate (/opt/app-root/src/node_modules/got/index.js:264:8)
  | at runCallback (timers.js:637:20)
  | at tryOnImmediate (timers.js:610:5)
  | at processImmediate [as _immediateCallback] (timers.js:582:5)

[panva]

the feature branch results in the following being accepted

Issuer.httpClient = {
   get(url, options) {}, // return Promise
   post(url, options) {}, // return Promise
   HTTPError, // used error constructor
};

What you fit those with is up to you. You may find the mapping of got to request in the original link @JoshBarr posted. This is definitely a more advanced topic so only get yourself on this path when necessary.

[big8extreme]

@JoshBarr have you an exemple for using the wrapper, In the company we hace to use a proxy and got not working very well with proxy.

[big8extreme]

@panva , its working, the wrapper of @JoshBarr is not working.
this exemple works well for me:

const request = require('request');
const HttpError = require('http-error-constructor');
/*
 * url {String}
 * options {Object}
 * options.headers {Object}
 * options.body {String|Object}
 * options.query {Object}
 * options.timeout {Number}
 * options.retries {Number}
 * options.followRedirect {Boolean}
 */

module.exports.get = function get(url, options) {
    options.uri = url;
    return new Promise((resolve, reject) => {
        request.get(options, (error, response, body) => {
            if(error) reject(err);
            resolve(response);
        });
    });
};

module.exports.post = function post(url, options) {
    options.uri = url;
    delete options.from;
    const body = options.body;
    delete options.body;
    options.form = body;
    return new Promise((resolve, reject) => {
        request.post(options, (error, response, body) => {
            if(error) reject(err);
            resolve(response);
        });
    });
};

module.exports.HTTPError = HttpError;

and added this in my app.js file, just before the issuer discover

const wrapper = require('./utils/wrapper');
Issuer.httpClient = wrapper;

and now the http_proxy works very well.

[panva]

The tests don't pass with your httpClient in, since this is a recurring topic let me see if i can accomodate a 100% working one myself.

[panva]

Here's a working request example that passes all tests.

'use strict';

const request = require('request');
const http = require('http');

class HTTPError extends Error {
  constructor(response) {
    let statusMessage;
    if (response.statusMessage) {
      statusMessage = response.statusMessage.replace(/\r?\n/g, ' ').trim();
    } else {
      statusMessage = http.STATUS_CODES[response.statusCode];
    }
    super(`Response code ${response.statusCode} (${statusMessage})`, {});
    this.name = 'HTTPError';
    this.statusCode = response.statusCode;
    this.statusMessage = statusMessage;
    this.headers = response.headers;
    this.response = response;
  }
}

function requestWrap(method, url, options) {
  if (options.form) {
    options.form = options.body;
    delete options.body;
  }
  return new Promise((resolve, reject) => {
    request({
      method,
      url,
      headers: options.headers,
      qs: options.query,
      body: options.body,
      form: options.form,
      followRedirect: options.followRedirect,
      timeout: options.timeout,
    }, (error, response, body) => {
      if (error) {
        reject(error);
      } else {
        response.body = body;
        const statusCode = response.statusCode;
        const limitStatusCode = options.followRedirect ? 299 : 399;

        if (statusCode !== 304 && (statusCode < 200 || statusCode > limitStatusCode)) {
          reject(new HTTPError(response));
          return;
        }

        resolve(response);
      }
    });
  });
}

/*
 * url {String}
 * options {Object}
 * options.headers {Object}
 * options.body {String|Object}
 * options.form {Boolean}
 * options.query {Object}
 * options.timeout {Number}
 * options.retries {Number}
 * options.followRedirect {Boolean}
 */
module.exports.get = function get(url, options) {
  return requestWrap('GET', url, options);
};

module.exports.post = function post(url, options) {
  return requestWrap('POST', url, options);
};

module.exports.HTTPError = HTTPError;

[panva]

Calling for comments in #65