Search code examples
node.jssequelize.jsmysql2node-mysqldecrement

better solution for sequelize decrement


I'm new to sequelize & trying to do decrement. I've two models products & order, in product there is productStock column & in order there is quantity column. What I'm trying to do is when I create new order the productStock will be decreased depends on ordered quantity. Code as below:

router.post('/', async (req, res) => {
try {
    const { productId, quantity } = req.body
    const postData = new Order({
        productId, 
        quantity
    })


    const resultdata = await Product.findOne({ where: {productId: postData.productId} }).then( product => {
        if(product.productStock <= 0) {
            return res.json({
                status: "failed",
                message: "out of stock"
            })
        } else {
            product.decrement('productStock', {by: postData.quantity})
            return postData.save()
        }
        
    } ).catch(err => err)

    res.send({
        status: "success",
        data: resultdata
    })

} catch (err) {
    throw err
}
 })

I got the result that I wanted, but the problem is when productStock <= 0 the result is correct, but I receive error in terminal as below:

(node:13236) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Socket'
|     property '_writableState' -> object with constructor 'WritableState'
|     property 'afterWriteTickInfo' -> object with constructor 'Object'
--- property 'stream' closes the circle
at JSON.stringify (<anonymous>)
at stringify (D:\Projects\sequelize-association\node_modules\express\lib\response.js:1123:12)
at ServerResponse.json (D:\Projects\sequelize-association\node_modules\express\lib\response.js:260:14)
at ServerResponse.send (D:\Projects\sequelize-association\node_modules\express\lib\response.js:158:21)
at D:\Projects\sequelize-association\routes\orders.js:30:13
at processTicksAndRejections (internal/process/task_queues.js:93:5)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:13236) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated 
either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:13236) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Please anyone help, also I think my code is not clean due to using promises then/catch inside try/catch, so if there are better solution for my case it will be great thanks....


Solution

  • I think the error is occurring when resultData is returned in res.send(...). It probably has a circular reference in it somewhere. To avoid this, one option would be to return something like JSON.stringify(resultData), which would prevent the error, although I'm not sure how useful it would be....

    A couple of things to note:

    • The suggested way to create new objects against the database is by using the .create() method or the .build() method followed by .save(), at least in version 6 of sequelize (creating new instances). I'm actually not sure what would happen if values are just passed into the constructor, as in new Order({ productId, quantity }), but updating the database would have to be asynchronous, and object instantiations are not meant to be asynchronous in javascript.
    • .catch(err => err) probably won't handle the error the way you want.
    • When product.productStock <= 0, the value of res.json(...) is returned and stored in resultData. Subsequently, resultData is returned in res.send(...), which is not allowed in express.js. In other words, there cannot be two responses for the same http request.
    • When there are a series of steps to be executed against the database that need to be made atomic, sequelize offers transactions, which are helpful.

    With this in mind, I would try refactoring to something like the code below. There may be additional ways to improve things, but at least the error should go away, and some of the above problems will be fixed.

    const sequelize = new Sequelize(/* connection string or other db options here */)
    
    router.post('/', async (req, res) => {
    
        const { productId, quantity } = req.body
    
        let t = await sequelize.transaction()
    
        try {
            let order,
                product
    
            product = await Product.findOne({
                    where: { productId },
                    transaction: t
                })
    
            if (!product || product.productStock <= 0) {
                await t.rollback()
                res.json({
                        status: "failed",
                        message: "out of stock"
                    })
            } else {
    
                order = await Order.create({
                        productId, 
                        quantity
                    }, {
                        transaction: t
                    })
    
                await product.decrement('productStock', {
                        by: order.quantity,
                        transaction: t
                    })
    
                await t.commit()
    
                res.send({
                    status: "success",
                    data: JSON.stringify(order)
                })
            }
        } catch (err) {
            if (t) {
                await t.rollback()
            }
        }
    })