Fix #1478 - Project files are not being flushed to disk after being written (#1479)

* Call fsync() before closing file

* Fix race condition in tests due to incorrect stub.

The startFlows() function wasn't really being stubbed, so it was still being called. But there was no corresponding call to stopFlows().

In later tests, the check in Flows.init() was throwing the "Cannot init without a stop" error.

* Test coverage for fsync() calls

For issue #1478

* Revert "Fix race condition in tests due to incorrect stub."

This reverts commit 4f71d7851b.

* Fix race condition in tests due to incorrect stub.

The startFlows() function wasn't really being stubbed, so it was still being called. But there was no corresponding call to stopFlows().

In later tests, the check in Flows.init() was throwing the "Cannot init without a stop" error.

* Fix intermittent test failure in Exec node.

Occasionally, the error text on stderr will come in more than one piece. The test only worked correctly if a single message was received.

red/runtime/locales/en-US/runtime.json

@@ -137,7 +137,8 @@
             "empty": "Existing __type__ file is empty",
             "invalid": "Existing __type__ file is not valid json",
             "restore": "Restoring __type__ file backup : __path__",
-            "restore-fail": "Restoring __type__ file backup failed : __message__"
+            "restore-fail": "Restoring __type__ file backup failed : __message__",
+            "fsync-fail": "Flushing file __path__ to disk failed : __message__"
         }
     }
 }

red/runtime/storage/localfilesystem.js

@@ -114,8 +114,13 @@ function writeFile(path,content) {
     return when.promise(function(resolve,reject) {
         var stream = fs.createWriteStream(path);
         stream.on('open',function(fd) {
-            stream.end(content,'utf8',function() {
+            stream.write(content,'utf8',function() {
-                fs.fsync(fd,resolve);
+                fs.fsync(fd,function(err) {
+                    if (err) {
+                        log.warn(log._("storage.localfilesystem.fsync-fail",{path: path, message: err.toString()}));
+                    }
+                    stream.end(resolve);
+                });
             });
         });
         stream.on('error',function(err) {

test/nodes/core/core/75-exec_spec.js

@@ -544,6 +544,7 @@ describe('exec node', function() {
         it('should return an error for a failing command', function(done) {
             var flow;
             var expected;
+            var expectedFound = false;
             if (osType === "Windows_NT") {
                 // Cannot use mkdir because Windows mkdir command automatically creates non-existent directories.
                 flow = [{id:"n1",type:"exec",wires:[["n2"],["n3"],["n4"]],command:"ping /foo/bar/doo/dah", addpay:false, append:"", useSpawn:"true", oldrc:"false"},
@@ -563,12 +564,18 @@ describe('exec node', function() {
                     try {
                         msg.should.have.property("payload");
                         msg.payload.should.be.a.String();
-                        msg.payload.indexOf(expected).should.not.be.equal(-1);
+                        if (msg.payload.indexOf(expected) >= 0) {
+                            // The error text on the stderr stream might get sent in more than one piece.
+                            // We only need to know that it occurred before the return code is sent,
+                            // as checked below in node n4.
+                            expectedFound = true;
+                        }
                     }
                     catch(err) { done(err); }
                 });
                 n4.on("input", function(msg) {
                     try {
+                        expectedFound.should.be.true;
                         msg.should.have.property("payload");
                         msg.payload.should.have.property("code",1);
                         done();

test/red/runtime/nodes/index_spec.js

@@ -26,11 +26,11 @@ var registry = require("../../../../red/runtime/nodes/registry");
 
 describe("red/nodes/index", function() {
     before(function() {
-        sinon.stub(flows,"startFlows");
+        sinon.stub(index,"startFlows");
         process.env.NODE_RED_HOME = path.resolve(path.join(__dirname,"..","..","..",".."))
     });
     after(function() {
-        flows.startFlows.restore();
+        index.startFlows.restore();
         delete process.env.NODE_RED_HOME;
     });
 

test/red/runtime/storage/localfilesystem_spec.js

@@ -17,8 +17,10 @@
 var should = require("should");
 var fs = require('fs-extra');
 var path = require('path');
+var sinon = require('sinon');
 
 var localfilesystem = require("../../../../red/runtime/storage/localfilesystem");
+var log = require("../../../../red/runtime/log");
 
 describe('LocalFileSystem', function() {
     var userDir = path.join(__dirname,".testUserHome");
@@ -279,6 +281,45 @@ describe('LocalFileSystem', function() {
         });
     });
 
+    it('should fsync the flows file',function(done) {
+        var flowFile = 'test.json';
+        var flowFilePath = path.join(userDir,flowFile);
+        localfilesystem.init({userDir:userDir, flowFile:flowFilePath}).then(function() {
+            sinon.spy(fs,"fsync");
+            localfilesystem.saveFlows(testFlow).then(function() {
+                fs.fsync.callCount.should.eql(1);
+                fs.fsync.restore();
+                done();
+            }).otherwise(function(err) {
+                done(err);
+            });
+        }).otherwise(function(err) {
+            done(err);
+        });
+    });
+
+    it('should log fsync errors and continue',function(done) {
+        var flowFile = 'test.json';
+        var flowFilePath = path.join(userDir,flowFile);
+        localfilesystem.init({userDir:userDir, flowFile:flowFilePath}).then(function() {
+            sinon.stub(fs,"fsync", function(fd, cb) {
+                cb(new Error());
+            });
+            sinon.spy(log,"warn");
+            localfilesystem.saveFlows(testFlow).then(function() {
+                log.warn.callCount.should.eql(1);
+                log.warn.restore();               
+                fs.fsync.callCount.should.eql(1);
+                fs.fsync.restore();
+                done();
+            }).otherwise(function(err) {
+                done(err);
+            });
+        }).otherwise(function(err) {
+            done(err);
+        });
+    });
+
     it('should backup the flows file', function(done) {
         var defaultFlowFile = 'flows_'+require('os').hostname()+'.json';
         var defaultFlowFilePath = path.join(userDir,defaultFlowFile);