Search code examples
javascriptpromisewinjs

Best practice to avoid clashing promises in js


I have a save function in my app which can be called manually and an autosave function which runs every 60 seconds.

To prevent the two ops trying to access the same file at the same instant, I set a flag called isSaving to true when one starts running, and to false again afterward. If open or save detect that autosave is running, they wait 1000ms and try again. If they fail after that I consider it an error.

Autosave:

setInterval(autosave, 1000 * 60);
isSaving = false;

function autosave() 
{        
    return new WinJS.Promise(function (complete, error, progress) 
    {
        if(isSaving == false) // no saving op in progress
        {
            // set saving flag on
            isSaving = true;
            // write file
            return writeFile(currentFile)
                .then(function () {
                    // saving flag off
                    isSaving = false;
                    complete();
                });
        }    
        else {
            // silently abort
            complete();
        }        
    });
}

Manual save:

var saveFileAttempts = 0;
function save()
{
    return new WinJS.Promise(function (complete, error, progress)
    {
        if (isSaving == false) // no saving op in progress
        {
            // set saving flag on
            isSaving = true;
            // write file
            return writeFile(currentFile)
                .then(function () {
                    // show notification to user "file saved"
                    return showSaveNotification()
                })
                .then(function () {
                    // set saving flag off
                    isSaving = false;
                    complete();
                });
        }
        else if (saveFileAttempts < 10) { 
            // try again in 1000ms, up to 10 times
            saveFileAttempts++;
            setTimeout(function () { save(); }, 1000);
        }
        else{
            error();
        }
    });
}

Open:

var openFileAttempts = 0;
function open()
{
    return new WinJS.Promise(function (complete, error, progress)
    {
        if (isSaving == false)
        {
            return readFile()
                .then(function (file) {
                    currentFile = file;
                    openFileAttempts = 0;
                    complete();
                });
        }
        else if (openFileAttempts < 10) { 
            // try again in 1000ms, up to 10 times
            openFileAttempts++;
            setTimeout(function () { open(); }, 1000);
        }
        else{
            error();
        }
    });
}

This feels like a hack. Is there a better way to achieve what I'm trying to do?

FYI: These functions return promises because there are other functions that call them.


Solution

  • Instead of waiting 1000ms and trying again, I'd recommend using a promise to represent that a save is ongoing and when it will end.

    var saving = null;
    
    setInterval(function() {
        if (!saving) // ignore autosave when already triggered
            save().then(showAutoSafeNotification);
    }, 60e3);
    
    function save() {
        if (saving)
            return saving.then(save); // queue
        // else
        var written = writeFile(currentFile);
        saving = written.then(function() {
            saving = null;
        }, function() {
            saving = null;
        });
        return written;
    }
    

    You can do the same with open (and might want to abstract the written part out), although I fail to see how it interferes with an (auto)save. If you're concerned about reading the file that is already open while it is saved, I'd let the filesystem handle that and catch the error.