Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report-processor uses an unholy mix of promises and callbacks #6

Open
ottumm opened this issue Sep 15, 2015 · 1 comment
Open

report-processor uses an unholy mix of promises and callbacks #6

ottumm opened this issue Sep 15, 2015 · 1 comment

Comments

@ottumm
Copy link
Contributor

ottumm commented Sep 15, 2015

processReport takes a callback but calls it from within a promise. This can lead to race conditions and generally difficult-to-follow behavior. processReport should implement a promise interface instead.

@briangreenery
Copy link
Contributor

It would take some refactoring, but here's what it seems like that file is doing and how functionality could be shifted around to possibly make the file more holy.

var parseReport = require('./report-parser.js');
var _ = require('lodash');

function getStatus(report) {
  if (report.bulkAggregates === undefined) {
    return 'unknown';
  }

  if (report.bulkAggregates.max === 0) {
    return 'failed';
  }

  return 'ok';
}

function addReport(doc, db, webHooks) {
  if (doc.monitors_id) {
    db.updateMonitorStatus(doc.monitors_id, doc.status);
  }

  return db.insertReport(doc)
    .then(function() {
      if (doc.owner) {
        webHooks.send(doc.owner, doc.report);
      }
    });
}

function processReport(doc, db, webHooks) {
  doc = _.clone(doc);
  doc.report = parseReport(doc.content);
  doc.status = getStatus(doc.report);

  if (doc.report.uuid === undefined) {
    throw new Error('Invalid report: missing UUID');
  }

  return db.getOrCreateMonitor(doc.report.uuid)
    .then(function(id) {
      doc.monitors_id = id;
      return addReport(doc, db, webHooks);
    });
}

module.exports = processReport;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants