A moratorium on a series of mistakes.

I recently emplaced a solution for logging on hapi services in Pacifica

It was not smooth sailing

proxy(request, reply) {
    return reply.proxy({
      mapUri: (req, cb) => cb(null, `${this.baseUrl}${req.url.path}`, ServiceCaller.scrubHeaders(req.headers)),
      onResponse: (err, res, req, rep) => {
        if (err) {
          rep(err.output.payload).code(err.output.statusCode);
        } else {
          rep(res);
        }
      }
    });
  }

service-caller.js

My first attempt at solving this problem.

proxy(request, reply) {
    const mapUri = setMapUri(this.baseUrl);
    return reply.proxy({
      mapUri,
      onResponse
    });
  }

function onResponse(error, response, request, reply) {
  return wreck.read(response, { json: true }, (err, payload) => {
    if (err) return reply(err);
    if (payload && ajv.validate(schema, payload)) {
      return reply(create(payload.statusCode, payload.message));
    }

    return reply(payload).code(response.statusCode);
  });
}

so I added all this crap...

const Ajv = require('ajv');

const ajv = new Ajv();

const schema = {
  maxProperties: 3,
  required: ['statusCode', 'error', 'message'],
  properties: {
    statusCode: {
      type: 'number',
      minimum: 400
    },
    error: {
      type: 'string'
    },
    message: {
      type: 'string'
    }
  }
};

Until some one tried to send an array through here.

function onResponse(error, response, request, reply) {
  return wreck.read(response, { json: true }, (err, payload) => {
    const isBoomError = ajv.validate(schema, payload);
    if (err) return reply(err);

    if (payload && !is(Array, payload) && isBoomError) {
      return reply(create(payload.statusCode, payload.message));
    }

    return reply(payload).code(response.statusCode);
  });
}

function onResponse(error, response, request, reply) {
  return wreck.read(response, { json: true }, (err, payload) => {
    if (err) return reply(err);

    if (response.statusCode >= BAD_REQUEST) {
      return reply(create(payload.statusCode, payload.message));
    }

    return reply(payload).code(response.statusCode);
  });
}

'use strict';

const Ajv = require('ajv');
const niceRequest = require('nice-request');
const _ = require('lodash');
const { wrap, create } = require('boom');
const wreck = require('wreck');
const { is } = require('ramda');

const ajv = new Ajv();

const schema = {
  maxProperties: 3,
  required: ['statusCode', 'error', 'message'],
  properties: {
    statusCode: {
      type: 'number',
      minimum: 400
    },
    error: {
      type: 'string'
    },
    message: {
      type: 'string'
    }
  }
};

class ServiceCaller {
  constructor({ locator, credentials, headers, tag }) {
    this.locator = locator;
    this.headers = ServiceCaller.scrubHeaders(headers);
    this.credentials = credentials;
    this.tag = tag;
    this.baseUrl = `http://${locator.host}:${locator.port}`;
  }

  static createFromRequest(request, locator, tag) {
    return new ServiceCaller({
      locator,
      tag,
      headers: request.headers,
      credentials: request.auth.credentials
    });
  }

  static scrubHeaders(headers) {
    // whitelist specific headers
    return _.pick(headers, [
      'user-agent',
      'host',
      'mfauth',
      'x-request-id'
    ]);
  }

  appendTag(tag) {
    if (!tag) return null;
    if (!this.tag) return tag;
    return [this.tag, tag].join('.');
  }

  callService({ method, path, headers, payload, tag }) {
    const opts = {
      method,
      url: `${this.baseUrl}${path}`,
      metricTag: this.appendTag(tag),
      body: payload,
      headers: Object.assign({}, this.headers, ServiceCaller.scrubHeaders(headers))
    };

    if (this.credentials) opts.headers.mfauth = JSON.stringify(this.credentials);
    return niceRequest.request(opts)
    .catch(error => {
      throw wrap(error, error.statusCode, error.message);
    });
  }

  // convenience methods to reduce opt call requirements

  get(opts) {
    return this.callService(Object.assign({ method: 'GET' }, opts));
  }

  put(opts) {
    return this.callService(Object.assign({ method: 'PUT' }, opts));
  }

  post(opts) {
    return this.callService(Object.assign({ method: 'POST' }, opts));
  }

  del(opts) {
    return this.callService(Object.assign({ method: 'DELETE' }, opts));
  }

  proxy(request, reply) {
    const mapUri = setMapUri(this.baseUrl);
    return reply.proxy({
      mapUri,
      onResponse
    });
  }
}

function setMapUri(baseUrl) {
  return (request, cb) => cb(null, `${baseUrl}${request.url.path}`, ServiceCaller.scrubHeaders(request.headers));
}

function onResponse(error, response, request, reply) {
  return wreck.read(response, { json: true }, (err, payload) => {
    const isBoomError = ajv.validate(schema, payload);
    if (err) return reply(err);

    if (!is(Array, payload) && isBoomError) {
      return reply(create(payload.statusCode, payload.message));
    }

    return reply(payload).code(response.statusCode);
  });
}

exports.setMapUri = setMapUri;
exports.onResponse = onResponse;
exports.ServiceCaller = ServiceCaller;

3rd version. 

2nd version

'use strict';

const Ajv = require('ajv');
const niceRequest = require('nice-request');
const _ = require('lodash');
const { wrap, create } = require('boom');
const wreck = require('wreck');

const ajv = new Ajv();

const schema = {
  maxProperties: 3,
  required: ['statusCode', 'error', 'message'],
  properties: {
    statusCode: {
      type: 'number',
      minimum: 400
    },
    error: {
      type: 'string'
    },
    message: {
      type: 'string'
    }
  }
};

class ServiceCaller {
  constructor({ locator, credentials, headers, tag }) {
    this.locator = locator;
    this.headers = ServiceCaller.scrubHeaders(headers);
    this.credentials = credentials;
    this.tag = tag;
    this.baseUrl = `http://${locator.host}:${locator.port}`;
  }

  static createFromRequest(request, locator, tag) {
    return new ServiceCaller({
      locator,
      tag,
      headers: request.headers,
      credentials: request.auth.credentials
    });
  }

  static scrubHeaders(headers) {
    // whitelist specific headers
    return _.pick(headers, [
      'user-agent',
      'host',
      'mfauth',
      'x-request-id'
    ]);
  }

  appendTag(tag) {
    if (!tag) return null;
    if (!this.tag) return tag;
    return [this.tag, tag].join('.');
  }

  callService({ method, path, headers, payload, tag }) {
    const opts = {
      method,
      url: `${this.baseUrl}${path}`,
      metricTag: this.appendTag(tag),
      body: payload,
      headers: Object.assign({}, this.headers, ServiceCaller.scrubHeaders(headers))
    };

    if (this.credentials) opts.headers.mfauth = JSON.stringify(this.credentials);
    return niceRequest.request(opts)
    .catch(error => {
      throw wrap(error, error.statusCode, error.message);
    });
  }

  // convenience methods to reduce opt call requirements

  get(opts) {
    return this.callService(Object.assign({ method: 'GET' }, opts));
  }

  put(opts) {
    return this.callService(Object.assign({ method: 'PUT' }, opts));
  }

  post(opts) {
    return this.callService(Object.assign({ method: 'POST' }, opts));
  }

  del(opts) {
    return this.callService(Object.assign({ method: 'DELETE' }, opts));
  }

  proxy(request, reply) {
    const mapUri = setMapUri(this.baseUrl);
    return reply.proxy({
      mapUri,
      onResponse
    });
  }
}

function setMapUri(baseUrl) {
  return (request, cb) => cb(null, `${baseUrl}${request.url.path}`, ServiceCaller.scrubHeaders(request.headers));
}

function onResponse(error, response, request, reply) {
  return wreck.read(response, { json: true }, (err, payload) => {
    if (err) return reply(err);
    if (payload && ajv.validate(schema, payload)) {
      return reply(create(payload.statusCode, payload.message));
    }

    return reply(payload).code(response.statusCode);
  });
}

exports.setMapUri = setMapUri;
exports.onResponse = onResponse;
exports.ServiceCaller = ServiceCaller;

Original Version Original Problem

const niceRequest = require('nice-request');
const _ = require('lodash');

class ServiceCaller {
  constructor({ locator, credentials, headers, tag }) {
    this.locator = locator;
    this.headers = ServiceCaller.scrubHeaders(headers);
    this.credentials = credentials;
    this.tag = tag;
    this.baseUrl = `http://${locator.host}:${locator.port}`;
  }

  static createFromRequest(request, locator, tag) {
    return new ServiceCaller({
      locator,
      tag,
      headers: request.headers,
      credentials: request.auth.credentials
    });
  }

  static scrubHeaders(headers) {
    // whitelist specific headers
    return _.pick(headers, [
      'user-agent',
      'host',
      'mfauth'
    ]);
  }

  appendTag(tag) {
    if (!tag) return null;
    if (!this.tag) return tag;
    return [this.tag, tag].join('.');
  }

  callService({ method, path, headers, payload, tag }) {
    const opts = {
      method,
      url: `${this.baseUrl}${path}`,
      metricTag: this.appendTag(tag),
      body: payload,
      headers: Object.assign({}, this.headers, ServiceCaller.scrubHeaders(headers))
    };

    if (this.credentials) opts.headers.mfauth = JSON.stringify(this.credentials);

    return niceRequest.request(opts);
  }

  // convenience methods to reduce opt call requirements

  get(opts) {
    return this.callService(Object.assign({ method: 'GET' }, opts));
  }

  put(opts) {
    return this.callService(Object.assign({ method: 'PUT' }, opts));
  }

  post(opts) {
    return this.callService(Object.assign({ method: 'POST' }, opts));
  }

  del(opts) {
    return this.callService(Object.assign({ method: 'DELETE' }, opts));
  }

  proxy(request, reply) {
    return reply.proxy({
      mapUri: (req, cb) => cb(null, `${this.baseUrl}${req.url.path}`, ServiceCaller.scrubHeaders(req.headers)),
      onResponse: (err, res, req, rep) => {
        if (err) {
          rep(err.output.payload).code(err.output.statusCode);
        } else {
          rep(res);
        }
      }
    });
  }
}

module.exports = ServiceCaller;

Mistake # Too large of a PR

Title Text

• PR too large - Set reviewers up for failure. 
• Mixed fixes in with current FB instead of doubling back to main rebranching, fixing, merging, then rebasing my FB back onto main after fix was accepted.
• Because fixes where mixed in with larger unrelated body of work no one was able to have a proper discussion about my fix. No one asked the golden question, "What problem am I trying to solve."

Weak Process, no feed back.

• No one called me out for the large PR
• No one called me out for mixing fixes in with larger body of work.