Node js Express server cannot get after sometime

Question

The server is running and getting a lot of request while this route is not working all the other routes are working.

I am picking up error 404 with the following message.

my route works and after a while stops to work I don't know why and how this happens. code example:

`/images` route

/**
 * Root route, all validation will be done here.
 * TODO:
 */
const { insertImgs,getExisting,getImg } = require('../core/DB.js');
const logger = require( '../logger/logger.js' ).getInstance();

class Images {
    constructor(expressApp) {
        if (!expressApp) throw 'no express app';
        this.router = expressApp;
        this.configure();
    }

    configure() {
        this.router.get('/images',async (req, res, next) => {
            try {
                let images = await getImg(res.locals.config.siteConfig.ID, res.locals.config.siteConfig.ocrLicense)
                /* merge results. */
                return res.status(200).send( images );
            } catch (err) {
                /* log error */
                console.log( 'alts failed', err );
                logger.error({ 
                    type : 'req denied', 
                    route : 'alts', 
                    ip, 
                    key : req.body.key,
                    data : req.body, 
                    error : err 
                });
                return res.status(503).send('Server Maintenance.');
            }
        });
        return this.app;
    }
}
module.exports = {
    Images : Images
}

main.js

const express = require( "express" );
    const cors   = require( "cors" );
    const http   = require( 'http' );
    const config = require( './config.js' );
    const helmet = require("helmet");

    /* routes */
    const { Root } = require('./routes/root.js');
    const { Alt } = require('./routes/alts.js');
    const { Images } = require('./routes/images.js');
    const { Health } = require('./routes/health.js');

    const logger = require('./logger/logger.js').getInstance();

    const app = express();
    const server  = http.createServer(app);
    const port = config.port || 3003;

    app.use( express.json() );
    app.use( express.urlencoded( { extended: true } ) );
    app.use( cors() );
    app.use( helmet() );
    /**
     * init routes.
     */
    [   
        Root,
        Images,
        Alt,
        Health 
    ].forEach(route => {
        new route(app)
    });
    //404 last handler
    app.use((req, res, next)=> {
        res.status(404).send({error: 'Page not found'});
    });
    // error handler
    app.use(function(err, req, res, next) {
        // render the error page
        res.status(err.status || 500);
        res.send('error 404');
    });

    server.listen(port,()=>{
        logger.log({ type : `server startup in process : ${ process.pid }` , port : port });
    });

If it's working, and then not, then some part of your code is **modifying** the behaviour (I'd assume overriding the endpoint handler). It doesn't magically just stop because of some reason :) — Andrey Popov, Mar 31 '22 at 13:30
Check your DB component. Often, when a route is working then suddeny doesn't respond anymore, it can be caused by a fault in releasing connection. But this would not explain why you get a 404. — Romain V..., Apr 03 '22 at 15:03
I'm afraid express.js was not desinged to use this way. You could write it in a few simple lines, but you are wrapping routes by classes. What is the benefit of it? Epress.js is more about middleware and if you follow its pattern you will not get wired bugs. I think your objects were cleaned by the garbage collector as there are no references to them. — sultan, Apr 04 '22 at 23:20

score 1 · Accepted Answer · answered Mar 30 '22 at 13:08

i would guess that it has to do with the fact that you are using the http module in conjunction with express.

i also wouldn't wrap each route in an object, express provides a Router class to do exactly that:

i rewrote some parts of your code to be a bit more concise:

main.js

const express = require("express")
const cors = require("cors")
const helmet = require("helmet")

const config = require('./config.js')
const logger = require('./logger/logger.js').getInstance()
const images_route = require('./routes/images.js')

const app = express()
const port = config.port || 3003

function page_not_found(req, res, next) {
    return res.status(404).send({
        error: 'Page not found'
    })
}

function error_handler(err, req, res, next) {
    res.status(err.status || 500)
    return res.send('error 404')
}

app.use(
    express.json(),
    express.urlencoded({ extended: true }),
    cors(),
    helmet()
)

app.use('/images', image_route)
app.use(error_handler)
app.all('/', page_not_found)

app.listen(port, () => {
    logger.log({
        type: `server startup in process : ${ process.pid }`,
        port: port
    })
});

images.js

const express = require('express')
const route = express.Router()
const { insertImgs, getExisting, getImg } = require('../core/DB.js')
const logger = require('../logger/logger.js').getInstance()

route.get('/', async (req, res, next) => {
    try {
        let images = await getImg(
            res.locals.config.siteConfig.ID,
            res.locals.config.siteConfig.ocrLicense
        )

        return res
            .status(200)
            .send( images )
    } catch (err) {
        console.error('alts failed', err)

        logger.error({ 
                type: 'req denied', 
                route: 'alts', 
                ip, 
                key: req.body.key,
                data: req.body, 
                error: err 
        })

        return res
            .status(503)
            .send('Server Maintenance');
}
})

module.exports = route

it is much easier to see what routes are being used from your main.js file now, this makes it harder to have overlapping endpoints.

let me know if this helps at all.

Ok ill try to make the changes and will update – ShobiDobi Mar 30 '22 at 13:13 — ShobiDobi, Mar 30 '22 at 13:13

Besworks · Answer 2 · 2022-04-06T02:07:32.353

0

try/catch does not work with async code. You also should not be using await inside a route handler as doing so may prevent other requests from getting processed.

Instead of this code:

this.router.get('/images',async (req, res, next) => {
    try {
        let images = await getImg(res.locals.config.siteConfig.ID, res.locals.config.siteConfig.ocrLicense)
        /* merge results. */
        return res.status(200).send( images );
    } catch (err) {
        /* log error */
        console.log( 'alts failed', err );
        logger.error({ 
            type : 'req denied', 
            route : 'alts', 
            ip, 
            key : req.body.key,
            data : req.body, 
            error : err 
        });
        return res.status(503).send('Server Maintenance.');
    }
});

Try this modified version:

this.router.get('/images', (req, res) => {
  getImg(
    res.locals.config.siteConfig.ID,
    res.locals.config.siteConfig.ocrLicense
  ).then(images => {
    res.status(200).send(images);
  }).catch(err => {
    console.log('alts failed', err);
    logger.error({ 
        type : 'req denied', 
        route : 'alts', 
        ip, 
        key : req.body.key,
        data : req.body, 
        error : err 
    });
    res.status(503).send('Server Maintenance.');
  });
});

This assumes that getImg is either declared as an async function or returns a Promise.

edited Apr 06 '22 at 02:07

answered Apr 06 '22 at 00:05

Besworks

4,123
1
18
34

AFAIK, `try... catch...` can well work with async code as far as also the **`try... catch...`** block is itself inside an async function (like in this case), async functions in the block are correctly awaited and error catching is correctly managed (https://medium.com/itnext/error-handling-with-async-await-in-js-26c3f20bc06a). Agree with the other point you made, lots of requests to the *awaited* route certainly could block other requests. – cheesyMan Apr 06 '22 at 08:50
Sure, you can [use try/catch with await](https://stackoverflow.com/questions/40884153/try-catch-blocks-with-async-await) but then your code is no longer truly async and becomes blocking. And you definitely should not be blocking inside a route handler which is, I believe, the root of the original issue. – Besworks Apr 06 '22 at 13:41
Ok, I see what you're getting at, `await` only causes blocking inside the already `async` function and therefore does not block execution outside of it in the main thread. So try/catch should work. Perhaps `logger.error` is failing silently and `res.status(503)` never gets called... but usually in this case you'd eventually get a timeout error in the browser, not a 404. – Besworks Apr 06 '22 at 14:53
1

After looking closer I'm noticing this line is wrong: `app.use(function(err, req, res, next) {`. Middleware function parameters are `req`, `res` and `next` only. So `err` here becomes a reference the Request object, `req` is the Response object, and `res` is actually `next()`. So `res.status(err.status || 500)` is essentially trying to call a `status` method on the next middleware which doesn't exist. – Besworks Apr 06 '22 at 14:59
Great catch, @Besworks thumbs-up :) – cheesyMan Apr 06 '22 at 15:30

Node js Express server cannot get after sometime

/images route

main.js

2 Answers2

`/images` route