Moved fs.readFileSync within the try/catch

[ajbogh]

Moved readFileSync within the try/catch to prevent filesystem errors when a file was not present. The error when throws is true is the same as before, but when it's false the error is swallowed and the return is null.

[jprichardson]

Would you rebase and add a test? Thanks.

[ajbogh]

Sure.

[texels]

Any chance of this getting merged into the master branch?

[jprichardson]

@ajbogh please rebase & squash.

[ajbogh]

@jprichardson, please try now.

[TheLarkInn]

Bump for justice :-D

[RyanZim]

@jprichardson I'm not sure what to think of this.

On one hand, I see the point of making throws: false silence all errors.

On the other hand, silencing a filesystem error could hide a simple mistake like an incorrect path.

I, for one, have never used throws: false, so perhaps I'm missing the point.

[jprichardson]

This seems reasonable to me since throws is legacy (to this module) and if you explicitly opt-in and set throws to false, it feels very counterintuitive for it throw regardless of a JSON parse error or file error. I'm not sure that throws is even a good long-term solution, but it's what we have for now.

[Tarabass]

@RyanZim I have an user case for this. I try to read a file and when it doesn't exist I want to create one.

readFile: function(filePath, async) {
    var async = async === false ? false : true;

    if(async) {
        jsonfile.readFile(filePath, function(err, obj) {
            if(err) {
                return console.log(err);
            }

            return obj;
        });
    }
    else {
        // TODO: throws is not working. Issue is reported on github
        return jsonfile.readFileSync(filePath/*, { 'throws': false }*/);
    }
}

For now I had to catch the error, even if throws === false. All I wanted to do is checking if the function returns null so I can create one.

Old code:

readSmtpConfig: function() {
    var filePath = this.smtpConfigPath,
        smtpConfig;

    try {
        smtpConfig = fileUtil.readFile(filePath, false);
    }
    catch(e) {
        // ENOENT: no such file or directory
        if(e.code === 'ENOENT') {
            smtpConfig = {
                description: '',
                host: '',
                port: '',
                secure: true, // use SSL
                auth: {
                    user: '',
                    pass: ''
                },
                identity: {
                    name: '',
                    address: ''
                }
            };

            fileUtil.writeFile(filePath, smtpConfig, false);
        }
    }

    return smtpConfig;
}

Seems a bit odd to me that I have to catch an error which should not be thrown when throws === false.

With this fix a can simply read the file and when smtpConfig === null I create one. Catching the error is already done by fs. I should rely on jsonfile and asume that the file didn't exist when it returns null.