CODEWITHSUNDEEP
javascriptjsonnode.js
Alexander Mills
Alexander Mills

Reputation: 99970

Nested JSON.parse errors and JS object traversal errors not trapped by JS try/catch, crashes server

I have this POST route handler in Express. I am concerned that using one large try/catch block does not correctly trap all errors that occur inside it.

Inside the only try/catch block below, I try to access a nested native JS property - that being - parsed.template.list.push(listItem);

So I am trying to push an object onto an array in nested property. If template doesn't exist, it will cause a runtime error, but this isn't trapped by the try/catch, the server just quite literally halts, and no JSON response is sent.

Is there a way to improve my code - do I need to put a myriad of try/catches in my code to try to absent-mindedly safeguard everything?

 POST: function (req, res, next) {

            var incidentId = req.body.incident_id;

            if (incidentId) {   
                csDataHelper.getAccountNumWithIncidentCorrelationID(incidentId, function (err, response, body) {

                    if (err) {
                        return res.json({error: err.toString()});
                    }


                    try {  // big try/catch starts here

                        var body = JSON.parse(body);
                        var result = csDataHelper.parseInfoFromCSResponse(body);

                        if (!(result instanceof Error)) {

                            var accountNum = result.accountNum;
                            var homePhone = result.homePhone;
                            var altPhone = result.altPhone;

                            var absPath = path.resolve(nconf.get('globalRoot').concat('/json_response_templates/pay_by_phone.json'));
                            fs.readFile(absPath, 'utf8', function (err, jsonTemplate) {
                                if (err) {
                                    res.json({'error': err.toString()});
                                }
                                else {
                                    var str = jsonTemplate.replace('${accountNumber}', accountNum).replace('${incidentId}', incidentId);
                                    var parsed = JSON.parse(jsonTemplate);
                                    if (homePhone) {
                                        var listItem = {
                                            "label": homePhone,
                                        };
                                        parsed.template.list.push(listItem);
                                    }
                                    if (altPhone) {
                                        var listItem = {
                                            "label": altPhone,
                                        };
                                        parsed.template.list.push(listItem);
                                    }
                                    res.json(parsed);
                                }
                            });
                        }
                        else {
                            return res.send({error: 'no accountNumber sent to Baymax from Contextstore ->' + result});
                        }
                    }
                    catch (err) {
                        return res.json({error: err.toString()});
                    }

                });
            }
            else {
                res.send({error: 'null/empty incident_id posted to Baymax'});
            }
        }
    }

When certain code inside the try/catch experiences an error, the error is not trapped by the try/catch block. SPECIFICALLY, I know that in some cases "parsed.template.list.push(listItem);" should actually be "parsed.list.push(listItem);", depending on the nature of the JSON coming at me.

In other words, in JS, do I need to use nested try/catches when using nested JSON.parse calls or nested JS object property retrievals?

Frankly, this is quite dangerous and of all the things I have seen in Node.js, parsing JSON and traversing native JS objects poses some of the biggest threats to server uptime.

How to combat this?

Upvotes: 0

Views: 449

Answers (1)

mscdex
mscdex

Reputation: 106698

The reason it's crashing on parsed.template.list.push(listItem) is because that line is inside a callback for an asynchronous function call. try-catch blocks do not magically cover callbacks.

So your best bet at the moment is to either add a try-catch inside the callback or add a conditional like if (parsed.template && parsed.template.list) { .. }.

Upvotes: 1

Related Questions