CODEWITHSUNDEEP
javascriptnode.jsexpressserverbackend
Florian Ludewig
Florian Ludewig

Reputation: 5982

Node.js Express Server - Global Variable Issue

To get this out of the way: I am not really posting a bug in this question!

Issue
I recently found a problem with my Node.js and Express backend code, while making multiple requests from the front end at the same time. Let's assume one of my endpoints looks like this:

var payload, id1, id2, data1, data2

exports.someFunction = async (req, res) => {
    payload = req.body.payload
    id1 = req.params.id1
    id2 = req.query.id2
    try {
        data1 = await fetchData1()
        data2 = await fetchData2()
        return responseHandler.success({ data1, data2 })
    } catch (err) {
        return responseHandler.error(err)
    }
}

async function fetchData1() {
    return new Promise((resolve, reject) => {
        // fetch data from database by accessing
        // payload, id1, id2
        // here
    })
}

The issue I discovered, was that the global variables like payload, id1, etc. were overwritten while the async functions were executed. (If the next request is made before the first one finished) Thereby some functions were executed with wrong inputs and weird responses emerged.

Solution
My solution then was to move those global variables inside the function, which resulted in a heavy usage of function arguments:

exports.someFunction = async (req, res) => {
    const payload = req.body.payload
    const id1 = req.params.id1
    const id2 = req.query.id2
    try {
        const data1 = await fetchData1(payload, id1, id2)
        const data2 = await fetchData2(payload, id1, id2, data1)
        return responseHandler.success({ data1, data2 })
    } catch (err) {
        return responseHandler.error(err)
    }
}

async function fetchData1(payload, id1, id2) {
    return new Promise((resolve, reject) => {
        // fetch data from database
    })
}

As you can see, the code gets really messy, which actually was the reason I used global variables in the first place.

My actual questions

Upvotes: 0

Views: 327

Answers (2)

Anthony Garcia-Labiad
Anthony Garcia-Labiad

Reputation: 3711

(1) Is it silly to use "global variables" in express routes?

Global variables are indeed considered bad practice in general.

(2) Is there a better way of supplying other functions with data, rather than inputting all arguments every single time

What's wrong with inputing them every single time? The code you show seems perfectly fine to me. It's usually better for readability and testing to have the dependencies of your functions explicit.

(3) Is my hypothesis correct, that those "global variables" are overwritten when a new request calls this specific route?

Yes, javascript is executed synchronously by default, until an async/await block appears. In your example, there is no guarantee that the async block will be resolved before another request is made, which make that code very fragile.

Upvotes: 2

Danmoreng
Danmoreng

Reputation: 2367

  1. Depends on the usage of the global variables. If you have something that is valid for all requests but might change based on other conditions it makes totally sense to use global variables in my opinion. But for your use case it's absolutely the wrong approach as you already found out.
  2. No. The only alternative would be to pass an object with multiple properties instead of multiple arguments - but basically that's the same.
  3. Yes, since the variables are global and requests are asynchronous the variables will be overwritten each time. Depending on the amount of time your fetchData functions take your handler will use the wrong data.

Upvotes: 1

Related Questions