Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm working on converting a simple Python script into nodejs 4.4.5 with es6 features like Promises.

The original Python script did the following:

  • Contact a 3rd party SOAP service
  • Attempt to retrieve results by calling various SOAP methods
  • Display the results as a page that can be monitored by PRTG or similar tools.

If our page renders with a status of 200, everything is good in 3rd-party land. If anything else happens, there's a problem!

At this point I'm happy to get the code to work at all, but I'm sure there are many improvements that could be made. Without further ado, here's the code.

Setup:

const http = require('http');
const express = require('express');
// an external package to parse xml to json
// use this to parse the 3rd party's SOAP response XML
const parseString = require('xml2js').parseString;

// each object in testCases contains an array of hosts to test, a
// string of data to send, and an options object to pass to the http.request
const importedTestCases = require('./config.js').testCases;

const app = express();
app.set('json spaces', 4);

getThirdPartyData:

function getThirdPartyData(options, data) {
  // return a promise that will be resolved when the request is complete
  return new Promise(function httpRequestPromise(resolve, reject) {
    const req = http.request(options, (res) => {
      const responseData = [];
      console.log(`STATUS: ${res.statusCode}`);
      console.log(`HEADERS: ${JSON.stringify(res.headers)}`);
      res.setEncoding('utf8');
      res.on('data', (chunk) => {
        parseString(chunk, function handleXml(err, result) {
          if (err) {
            res.statusCode = err;
          } else {
            responseData.push(result);
          }
        });
      });
      // when the request is done receiving data, resolve the promise 
      // with a status code and some data
      res.on('end', () => { resolve({status: res.statusCode, data: responseData});});
    });
    req.on('error', (e) => {
      reject(`problem with request: ${e.message}`);
    });
    if (data) {
      req.write(data);
    }
    req.end();
  });
}

doTests:

function doTests(testCases) {
  // get response from third party for each test case and return array of results as promises
  const promises = [];
  for (const testcase of testCases) {
    const data = testcase.data;
    const hosts = testcase.hosts;
    const options = testcase.options;
    for (const host of hosts) {
      options.host = host;
      promises.push(getThirdPartyData(options, data));
    }
  }
  return promises;
}

Express route for root of site ('/'):

app.get('/', (req, res) => {
  // doTests on all the testcases
  // and resolve all the promises we get back
  Promise.all(doTests(importedTestCases))
  .then((responses) => {
    // after we resolve responses
    // create an array of only the status of each response
    const responseStatusArray = responses.map((response) => {
      return response.status;
    });
    // filter the statuses and find any that aren't 200 (OK)
    const not200 = responseStatusArray.filter((status) => {
      return status !== '200';
    });
    // our status will be either the first non-200 status from 3rd party, or else just a 200.
    const ourStatus = not200[0] || 200;
    // send the response with our status and a JSON array of the verbatim response from 3rd party
    res
      .status(ourStatus)
      .json(responses);
  }, (err) => {
    // if we got an error, set the status to something not 200
    // and display the error message.
    res
      .status(500)
      .json(err);
  });
});

Run Express on port 3000:

app.listen(3000, function listener() {
  console.log('listening on port 3000');
});

And an example of a testCases object:

const soapData = `<?xml version="1.0" encoding="UTF-8"?>
                            <SOAP-ENV:Envelope>... lots of SOAP junk ...
                            </SOAP-ENV:Envelope>`;

const soapAction = 'http://service.com/actionUrl';

const soapOptions = {
  // hostname will be inserted later for each host
  port: 80,
  path: 'http://service.com/service.wsdl',
  method: 'POST',
  headers: {
    'Accept-Encoding': 'identity',
    'Content-Type': 'text/xml; charset=utf-8',
    'Soapaction': soapAction,
  },
};

const host1 = 'host1.service.com';
const host2 = 'host2.service.com';

// just an example, the real file contains more than 1 object in testCases
const testCases = [
  {
    hosts: [host1, host2],
    options: soapOptions,
    data: soapData,
  },
];
exports = module.exports = {testCases: testCases};

My concerns are:

  1. Am I writing idiomatic JavaScript (for nodejs 4.4.5 and es6)?
    • Do I fall into the trap of writing any JavaScript anti-patterns?
  2. Is my code legible and succinct (within reason)?
  3. Am I approaching the problem in a logical manner?

I appreciate any constructive criticism!

share|improve this question

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.