Question 80289 in Javascript Question posted in
A very good W3school tutorial can be found here.

Using a function inside a try…catch block how to exit the block? – Javascript

When creating some endpoints using Node.js and express I came across a piece of code that is repeated a few times in the same controller file on different endpoints. This code is inside a conditional that is inside a try…catch block, it serves to validate if a sent parameter exists in the DB and so, if it exists or not, the execution proceeds accordingly. Here is my original code:

const productListing = async (req, res) => {
    const { category_id } = req.query;
    try {
        // if there are query params execute this if statement
        if (req.url.includes("?")) {
            // the next 3 lines of code repeat a few times throughout the controller

            const validCategory = await knex("categories").where("id", category_id)
            if (validCategory.length === 0) {
                return res.status(404).json({ "message": "Category not found" });
            };

            const productsByCategory = await knex("products").where({ category_id })
            return res.status(200).json(productsByCategory);
        }; 

        const allProducts = await knex("products")
        return res.status(200).json(allProducts);
    } catch (error) {
        return res.status(500).json({ "message": "Internal error" });
    };
};

In order to improve the code and avoid repetition I tried to create the function below and call it instead of the repetitive code, what I expected to get was similar behavior, but although the function works when no query params are sent, or when the category exists, when the category does not exist, it makes the node crash and does not give the proper return, because in this condition the function should finish the try…catch block but the node tries to set the headers after the function has already done that.

FUNCTION:

const validId = async (res, table, id) => {
    const validateId = await knex(table).where({ id })
        if (validateId.length === 0) {
            return res.status(404).json({ "message": `${table} not found` });
        };
    return 
};

MAIN CODE:

const productListing = async (req, res) => {
    const { category_id } = req.query;
    try {
        if (req.url.includes("?")) {
            validId(res, "categories", category_id);

            const productsByCategory = await knex("products").where({ category_id })
            return res.status(200).json(productsByCategory);
        }; 

        const allProducts = await knex("products")
        return res.status(200).json(allProducts);
    } catch (error) {
        return res.status(500).json({ "message": "Internal error" });
    };
};

ERROR CODE:

    > nodemon ./src/index.js
    
    [nodemon] 2.0.22
    [nodemon] to restart at any time, enter `rs`
    [nodemon] watching path(s): *.*
    [nodemon] watching extensions: js,mjs,json
    [nodemon] starting `node ./src/index.js`
    node:internal/errors:490
        ErrorCaptureStackTrace(err);
        ^
    
    Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
        at new NodeError (node:internal/errors:399:5)
        at ServerResponse.setHeader (node:_http_outgoing:663:11)
        at ServerResponse.header (D:CODINGCUBOSBACKENDunidade-5desafio-backend-final-dbe-b2b-t03-ifoodnode_modulesexpresslibresponse.js:794:10)
        at ServerResponse.send (D:CODINGCUBOSBACKENDunidade-5desafio-backend-final-dbe-b2b-t03-ifoodnode_modulesexpresslibresponse.js:174:12)
        at ServerResponse.json (D:CODINGCUBOSBACKENDunidade-5desafio-backend-final-dbe-b2b-t03-ifoodnode_modulesexpresslibresponse.js:278:15)
        at validaId (D:CODINGCUBOSBACKENDunidade-5desafio-backend-final-dbe-b2b-t03-ifoodsrcfuncoesvalidaId.js:6:36)
        at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
      code: 'ERR_HTTP_HEADERS_SENT'
    }
    
    Node.js v18.15.0
    [nodemon] app crashed - waiting for file changes before starting...
Tags:

2

Answers


  1. 0

    The

    return res.status(404).json({ "message": `${table} not found` });

    in the validId function is actually sending the response to the client but it never breaks execution of the function so then it goes on to the next

    return res.status(200).json(allProducts);

    and throws because the response has already been sent. You would need some way to determine that the response has already been sent by the validId function, if we kept things as they are currently. You could do something leveraging the res.headersSent property and not change too much of your code.

    const productListing = async (req, res) => {
    const { category_id } = req.query;
    try {
        if (req.url.includes("?")) {
            validId(res, "categories", category_id);
            if(res.headersSent) return;
            const productsByCategory = await knex("products").where({ category_id })
            return res.status(200).json(productsByCategory);
        }; 

        const allProducts = await knex("products")
        return res.status(200).json(allProducts);
    } catch (error) {
        return res.status(500).json({ "message": "Internal error" });
    };
};

    I think things become a bit more clear if you keep the validId function fairly limited in its responsibilities and leave the response handling to the productListing function.

    const validId = async (res, table, id) => {
    const validateId = await knex(table).where({ id })
    return validateId.length >= 0;
};

const productListing = async (req, res) => {
    const { category_id } = req.query;
    try {
        if (req.url.includes("?")) {
            if(!validId(res, "categories", category_id)) {
                return res.status(404).json({ "message": `Category not found` })
            }
            const productsByCategory = await knex("products").where({ category_id })
            return res.status(200).json(productsByCategory);
        }; 

        const allProducts = await knex("products")
        return res.status(200).json(allProducts);
    } catch (error) {
        return res.status(500).json({ "message": "Internal error" });
    };
};
    Login or Signup to reply.
  2. 0

    You forgot to await validId. Without await you can see that function actually gets executed after productListing returns. You can see validId was executed in the error stack trace – after a response was already sent.

    You also need to know whether or not validId already returned a response, so you don’t try to send a second response in the main function, resulting in the same error. You can do that by returning a boolean.

    const validId = async (res, table, id) => {
    const validateId = await knex(table).where({ id })
        if (validateId.length === 0) {
            res.status(404).json({ "message": `${table} not found` });
            return false;
        };
    return true;
};

    const productListing = async (req, res) => {
    const { category_id } = req.query;
    try {
        if (req.url.includes("?")) {
            // stop if not valid, validId will have responded already
            if (!(await validId(res, "categories", category_id)) return;

            const productsByCategory = await knex("products").where({ category_id })
            return res.status(200).json(productsByCategory);
        }; 

        const allProducts = await knex("products")
        return res.status(200).json(allProducts);
    } catch (error) {
        return res.status(500).json({ "message": "Internal error" });
    };
};
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search