1

I am using the restify framework to build a small app that copies an uploaded file from its temporary location to a permanent location and then inserts that new location into a MySQL database. However, when attempting to copy the file and then run the promisified query, the system throws a silent error not caught by the promise chain causing a 502 error on the web server end. A minimal working example is below. This example has been tested and does fail out of the gate.

If one of the steps in the process is removed (copying the file or storing the string in the database), the silent error disappears and API response is sent. However, both steps are needed for later file retrieval.

Main Restify File

const restify = require('restify');
const corsMiddleware = require('restify-cors-middleware');
const cookieParser = require('restify-cookies');

const DataBugsDbCredentials = require('./config/config').appdb;
const fs = require('fs');
const { host, port, name, user, pass } = DataBugsDbCredentials;
const database = new (require('./lib/database'))(host, port, name, user, pass);

const server = restify.createServer({
    name: 'insect app'
});

// enable options response in restify (anger) -- this is so stupid!! (anger)
const cors = corsMiddleware({});
server.pre(cors.preflight);
server.use(cors.actual);

// set query and body parsing for access to this information on requests
server.use(restify.plugins.acceptParser(server.acceptable));
server.use(restify.plugins.queryParser({ mapParams: true }));
server.use(restify.plugins.bodyParser({ mapParams: true }));
server.use(cookieParser.parse);


server.post('/test', (req, res, next) => {
    const { files } = req;

    let temporaryFile = files['file'].path;
    let permanentLocation = '/srv/www/domain.com/permanent_location';

    // copy file 
    return fs.promises.copyFile(temporaryFile, permanentLocation)

        // insert into database
        .then(() => database.query(
            `insert into Specimen (
                CollectorId,
                HumanReadableId,
                FileLocation
            ) values (
                1,
                'AAA004',
                ${permanentLocation}
            )`
        ))
        .then(() => {
            console.log('success!!!')
            return res.send('success!')
        })
        .catch(error => {
            console.error(error)
            return res.send(error);
        });
});

./lib/database.js

'use strict';

const mysql = require('mysql2');

class Database {
    constructor(host, port, name, user, pass) {
        this.connection = this.connect(host, port, name, user, pass);
        this.query = this.query.bind(this);
    }

    /**
     * Connects to a MySQL-compatible database, returning the connection object for later use
     * @param {String} host The host of the database connection
     * @param {Number} port The port for connecting to the database
     * @param {String} name The name of the database to connect to
     * @param {String} user The user name for the database
     * @param {String} pass The password for the database user
     * @return {Object} The database connection object
     */
    connect(host, port, name, user, pass) {
        let connection = mysql.createPool({
            connectionLimit : 20,
            host            : host,
            port            : port,
            user            : user,
            password        : pass,
            database        : name,
            // debug           : true
        });

        connection.on('error', err => console.error(err));
        return connection;
    }

    /**
     * Promisifies database queries for easier handling
     * @param {String} queryString String representing a database query
     * @return {Promise} The results of the query
     */
    query(queryString) {
        // console.log('querying database');
        return new Promise((resolve, reject) => {
            // console.log('query promise before query, resolve', resolve);
            // console.log('query promise before query, reject', reject);
            // console.log('query string:', queryString)
            this.connection.query(queryString, (error, results, fields) => {
                console.log('query callback', queryString);
                console.error('query error', error, queryString);
                if (error) {
                    // console.error('query error', error);
                    reject(error);
                } else {
                    // console.log('query results', results);
                    resolve(results);
                }
            });
        });
    }
}

module.exports = Database;

./testfile.js (used to quickly query the restify API)

'use strict';

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

let req = request.post({
    url: 'https://api.databugs.net/test',
}, (error, res, addInsectBody) => {
    if (error) {
        console.error(error);
    } else {
        console.log('addInsectBody:', addInsectBody);
    }
});
let form = req.form();
form.append('file', fs.createReadStream('butterfly.jpg'), {
    filename: 'butterfly.jpg',
    contentType: 'multipart/form-data'
});

If the request is made to the localhost, then an 'ECONNRESET' error is thrown as shown below:

Error: socket hang up
    at connResetException (internal/errors.js:570:14)
    at Socket.socketOnEnd (_http_client.js:440:23)
    at Socket.emit (events.js:215:7)
    at endReadableNT (_stream_readable.js:1183:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  code: 'ECONNRESET'
}

This error is only thrown if both the database and the file I/O are both present in the promise chain. Additionally, the error does not occur if the database request is made first with the file I/O occurring second; however, another rapid request to the server will immediately lead to the 'ECONNRESET' error.

Informagician
  • 305
  • 3
  • 15
  • This likely won't fix your problem, but there is a promise version of the pool, I'm surprised you're not using that instead of dealing with converting the callback to a promise manually. --- can you expand on " the system throws a silent error not caught by the promise chain causing a 502 error on the web server end" --- caught by what promise chain? You never set a 502 status and you never call next() so I only see that your `res.send(error);` would possibly send this 502. (Basically, where are you seeing this 502 and what is the output?) – Cody G Jan 07 '20 at 21:58
  • Thanks for the advice about promisified version of the pool; I hadn't seen that. The 502 error actually gets thrown by the nginx proxy server. The error shown at the bottom (socket hangup) is the actual error that is thrown by the nodejs server. The 502 is actually an nginx response to the upstream nodejs throwing that error. – Informagician Jan 10 '20 at 00:22

2 Answers2

1

I feel as though I should edit this answer, despite the solution revealing a rookie mistake, in the hopes that it may help someone else. I will keep the previous answer below for full transparency, but please not that it is incorrect.

Correct Answer

TL;DR

PM2 restarted the NodeJS service with each new file submitted to and saved by the API. The fix: tell PM2 to ignore the directory that stored the API's files. See this answer

Long Answer

While the OP did not mention it, my setup utilized PM2 as the NodeJS service manager for the application, and I had turned on the 'watch & reload' feature that restarted the service with each file change. Unfortunately, I had forgotten to instruct PM2 to ignore file changes in the child directory storing new files submitted through the API. As a result, each new file submitted into the API caused the service to reload. If more instructions remained to be executed after storing the file, they were terminated as PM2 restarted the service. The 502 gateway error was a simple result of the NodeJS service becoming temporarily unavailable during this time.

Changing the database transactions to occur first (as incorrectly described as a solution below) simply insured that the service restart occurred at the very end when no other instructions were pending.

Previous Incorrect Answer

The only solution that I have found thus far is to switch the file I/O and the database query so that the file I/O operation comes last. Additionally, changing the file I/O operation to rename rather than copy the file prevents rapidly successive API queries from throwing the same error (having a database query rapidly come after any file I/O operation that is not a rename seems to be the problem). Sadly, I do not have a reasonable explanation for the socket hang up in the OP, but below is the code from the OP modified to make it functional.

const restify = require('restify');
const corsMiddleware = require('restify-cors-middleware');
const cookieParser = require('restify-cookies');

const DataBugsDbCredentials = require('./config/config').appdb;
const fs = require('fs');
const { host, port, name, user, pass } = DataBugsDbCredentials;
const database = new (require('./lib/database'))(host, port, name, user, pass);

const server = restify.createServer({
    name: 'insect app'
});

// enable options response in restify (anger) -- this is so stupid!! (anger)
const cors = corsMiddleware({});
server.pre(cors.preflight);
server.use(cors.actual);

// set query and body parsing for access to this information on requests
server.use(restify.plugins.acceptParser(server.acceptable));
server.use(restify.plugins.queryParser({ mapParams: true }));
server.use(restify.plugins.bodyParser({ mapParams: true }));
server.use(cookieParser.parse);


server.post('/test', (req, res, next) => {
    const { files } = req;

    let temporaryFile = files['file'].path;
    let permanentLocation = '/srv/www/domain.com/permanent_location';

    // copy file 
    // insert into database
    return database.query(
            `insert into Specimen (
                CollectorId,
                HumanReadableId,
                FileLocation
            ) values (
                1,
                'AAA004',
                ${permanentLocation}
            )`
        )
        .then(() => fs.promises.rename(temporaryFile, permanentLocation))
        .then(() => {
            console.log('success!!!')
            return res.send('success!')
        })
        .catch(error => {
            console.error(error)
            return res.send(error);
        });
});
Informagician
  • 305
  • 3
  • 15
-1

You did not handle the database promise in then and catch -

Main Restify File


const restify = require('restify');
const corsMiddleware = require('restify-cors-middleware');
const cookieParser = require('restify-cookies');

const DataBugsDbCredentials = require('./config/config').appdb;
const fs = require('fs');
const { host, port, name, user, pass } = DataBugsDbCredentials;
const database = new (require('./lib/database'))(host, port, name, user, pass);

const server = restify.createServer({
    name: 'insect app'
});

// enable options response in restify (anger) -- this is so stupid!! (anger)
const cors = corsMiddleware({});
server.pre(cors.preflight);
server.use(cors.actual);

// set query and body parsing for access to this information on requests
server.use(restify.plugins.acceptParser(server.acceptable));
server.use(restify.plugins.queryParser({ mapParams: true }));
server.use(restify.plugins.bodyParser({ mapParams: true }));
server.use(cookieParser.parse);


server.post('/test', (req, res, next) => {
    const { files } = req;

    let temporaryFile = files['file'].path;
    let permanentLocation = '/srv/www/domain.com/permanent_location';

    // copy file 
    return fs.promises.copyFile(temporaryFile, permanentLocation)

        // insert into database
        .then(() =>{ 
                // Your database class instance query method returns promise 
                database.query(
                `insert into Specimen (
                    CollectorId,
                    HumanReadableId,
                    FileLocation
                ) values (
                    1,
                    'AAA004',
                    ${permanentLocation}
                )`
                ).then(() => {
                    console.log('success!!!')
                    return res.send('success!')
                })
                .catch(error => {
                    console.error('Inner database promise error', error)
                    return res.send(error);
                });
            }).catch(error => {
                console.error('Outer fs.copyfile promise error', error)
                return res.send(error);
            })

});

shiva2492
  • 409
  • 3
  • 10
  • I disagree with this answer for a few reasons. 1) While the database.query() function does indeed return a promise, .then() can accept a function that returns a promise, which will continue the promise chain. 2) It does not actually answer the question of getting the system to work and only proposes a mechanism for catching the error, which was added in an edit and is visible when querying the localhost – Informagician Jan 10 '20 at 00:30
  • Your solution has done the same thing its just you changed the order of execution and by no way I see this as not solving your problem but anyways good luck with your own answer acceptance. – shiva2492 Jan 10 '20 at 16:50
  • I did try the above solution before posting without any success. While the answer that I proposed does seem to do something simplistic (switching file I/O and the database query), this change works in practice. However, the solution proposed in the above answer is essentially identical to the code in the OP since a function returning a promise can be used in .then() without requiring its own .catch(). – Informagician Jan 11 '20 at 00:34