Strange (?) behaviour in q-io of open, copy when destination file is readonly

[Bruce Cartland]

Being new to this, I'm not sure of the expected behaviour. I'm running node 0.10.15 on Windows and q-io 1.10.3. 

If dst is writable, then everything works as expected. If, however, dst is readonly:

1. copy is completely silent - no message is output. The dst file is not affected. I would expect an error.

qfs.copy(src, dst)
.then(function () {
  console.log('copied file');
})
.catch(function (err) {
  console.error(err);
})
.finally(function () {
  console.log('copy finalized');
})
.done();

2. open reports success ('opened file' and 'open finalized') - I would have though "w" would result in an error

qfs.open(dst, {flags: "wb"})
.then(function () {
  console.log('opened file');
})
.catch(function (err) {
  console.error(err);
})
.finally(function () {
  console.log('open finalized');
})
.done();

3. write, in contrast, works as I expect - it generates an error and calls finally

qfs.write(dst, 'hello')
.then(function () {
  console.log('written file');
})
.catch(function (err) {
  console.error(err);
})
.finally(function () {
  console.log('write finalized');
})
.done();

thanks

[Kris Kowal]

Thanks. It looks like a bug to me, but I (or a friendly elf) will need to look deeper. If you have time to escalate this to the Q-IO issue tracker, I would be grateful.

--
You received this message because you are subscribed to the Google Groups "Q Continuum (JavaScript)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to q-continuum...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Bruce Cartland]

I'll raise an issue.

I worked out that it's because in writer.js, Writer() is returning Q(self) instead of a deferred 'begin' promise, as Reader does.

If I amend Writer, as below, then it will work correctly for both writable and non-writable files. However, I'm not sure if this solution will work for http streams as I'm not sure if such streams emit the 'open' event. Can you confirm this or point me in the correct direction? Happy to submit the fix if it's correct.

function Writer(_stream, charset) {
    var self = Object.create(Writer.prototype);

    if (charset && _stream.setEncoding) // TODO complain about inconsistency
        _stream.setEncoding(charset);

    var begin = Q.defer();
    var drained = Q.defer();

    _stream.on("error", function (reason) {
        begin.reject(reason);
    });
   
    _stream.on("open", function () {
        begin.resolve(self);
    });
   
    _stream.on("drain", function () {
        drained.resolve();
        drained = Q.defer();
    });

:.............. (rest of code)................


    return begin.promise;
    //return Q(self); // todo returns the begin.promise
}

[Bruce Cartland]

With the change (I also added the open event to Reader()), the Jasmine unit tests pass: 70 tests, 134 assertions, 0 failures, 0 skipped.

Another thing. Being pretty new to Promises, I can't judge if this is relevant, but I noticed Domenic's comment in https://groups.google.com/d/msg/q-continuum/Zk-ZsVEssCI/gA0rC1BG9uAJ

"we use Q.ninvoke to get into promise-land ASAP, and we thus never have to use deferreds, which are usually a sign something weird is going on. ".

In this scenario of handling events, do we have no choice but to use deferreds? Just curious.

[Kris Kowal]

Yes, adapting event oriented code into promises will always involve either using the promise constructor or using a deferred.

Kris