[1.0.0] Add better error trace on transform streams

[raix]

Issues like #225 are a bit hard to debug, there should be a better error message explaining that the Error toke place inside eg. transformWrite and hand the error message and trace if available.

• add error event listener on passthrough streams
  currently the error event is unhandled in the cfs-storage-adapter transform-stream.js

[ghost]

I've created something for this that could be included in the package. Just want your feedback before I refactor it into the cfs-storage-adapter.

How can the transformWrite function notify that an error has occured?

• The transformWrite function can throw an error that will be handled by the transformErrorHandler.
• The transformWrite function also receives a fourth argument. A function that can be called when an error occurs in transformWrite. Why?: Throwing errors inside event handlers (.on) doesn't do the right thing.

What will the error handler do?

• Save the error in file.copies[storageName].error. The frontend can check for this property and show the error to the user and/or delete the file for example.
• Emit an error on the writeStream, so the writeStream knows that it should abort.
• Unpipe the readStream. I don't know if this is necessary but it prevents further data transmission to the writeStream and the writeStream can handle the unpipe event.
• Emit the stored event (currently for compability)

Open

• @aldeed wrote I think we want to ultimately funnel the error back up to an onError handler on the collection. (Uploading corrupt images with gm transformWrite will never finish the upload #297 (comment)).

My current transformErrorHandler code

/**
 * Wrapper that handles errors that are thrown while transforming.
 * It saves the error in the copy and emits stored.
 * @param {string} storageName The name of the storage.
 * @param {Function} transformFunction The transformation function.
 * @returns {Function}
 */
var handleTransformErrors = function (storageName, transformFunction) {
  return function (file, readStream, writeStream) {
    var handleError = function (error) {
      if (_.isObject(error) && error.errorType === 'Meteor.Error') {
        error = _.pick(error, 'error', 'reason', 'details');
      }
      console.log('handleTransformError', error);
      file.copies[storageName].error = error;
      writeStream.emit('error', error);
      readStream.unpipe(writeStream);
      writeStream.emit('stored', {
        fileKey: '',
        size: 0,
        storedAt: new Date()
      });
    };
    try {
      transformFunction(file, readStream, writeStream, handleError);
    } catch (error) {
      handleError(error);
    }
  };
};

Usage

var eventPhotos = new FS.Store.S3('eventPhotos', {
  // Options...
  transformWrite: handleTransformErrors('eventPhotos', function (file, readStream, writeStream, handleError) {
    // Transform code.
    // Trigger error with throw:
    throw new Meteor.Error(500, 'Transform failed');
    // Or trigger error with callback:
    handleError(new Meteor.Error(500, 'Transform failed');
  })
});

[aldeed]

@raix may have comments, but I think your ideas generally make sense. If you want to put together a PR, we might have more comments on the actual implementation of it. The important point is to properly catch all errors and emit "error" instead of "stored". You don't have to worry about funneling the error back to the collection because we can do that when we add the collection emitter feature. I agree that adding an error property to copies[store] is probably the best way to get the error back to the client. It might actually be best to save the entire error object there rather than just the message. I think EJSON handles that properly, but not sure.

[piyushcoader]

Error: Error storing file to the Uploads store: socket hang up error and i have not used transformWrite

Error: Error storing file to the Uploads store: socket hang up
W20140824-11:14:45.598(5.75)? (STDERR) at null. (packages/cfs-collection/common.js:88)
W20140824-11:14:45.598(5.75)? (STDERR) at emit (events.js:106:17)
W20140824-11:14:45.599(5.75)? (STDERR) at Writable. (packages/cfs-storage-adapter/storageAdapter.server.js:203)
W20140824-11:14:45.599(5.75)? (STDERR) at Writable.emit (events.js:117:20)
W20140824-11:14:45.599(5.75)? (STDERR) at Response. (packages/cfs-s3/s3.upload.stream2.js:120)
W20140824-11:14:45.599(5.75)? (STDERR) at Request. (/Users/piyushthapa/.meteorite/packages/cfs-s3/CollectionFS/Meteor-cfs-s3/97d3c544165c766115f2b91a6aaca54a7438a558/.build/npm/node_modules/aws-sdk/lib/request.js:263:18)
W20140824-11:14:45.600(5.75)? (STDERR) at Request.callListeners (/Users/piyushthapa/.meteorite/packages/cfs-s3/CollectionFS/Meteor-cfs-s3/97d3c544165c766115f2b91a6aaca54a7438a558/.build/npm/node_modules/aws-sdk/lib/sequential_executor.js:117:20)

[cristiandley]

im having the same issue only under meteor-up

GET http://mydomain.com/sockjs/info?cb=7sbwqvwd9v net::ERR_CONNECTION_REFUSED 

Error: "Queue" network [undefined], Error
    at http://mydomain.com/d4113048f6873f06ed8b5ce655e8b1f5a39feb00.js:22:23751
    at http://mydomain.com/d4113048f6873f06ed8b5ce655e8b1f5a39feb00.js:22:21121
    at http://mydomain.com/d4113048f6873f06ed8b5ce655e8b1f5a39feb00.js:1:8368
    at XMLHttpRequest.v.onreadystatechange (http://mydomain.com/d4113048f6873f06ed8b5ce655e8b1f5a39feb00.js:22:22428) 

this is my issue issue on mup

[RobertLowe]

+1, this is really needed...

My natural thought would be that collection.insert(file, function(err)(){ }) would contain error(s) information.

I'd accept any error handling really...

PS: Has any work been started on this?

[evolross]

Hey Sanjo,

If I wanted to use your handleTransformErrors - how would I do that? Do you have a repository forked with it included - or can I edit it myself? Not sure where to put your function. And I'd really love to pass the error back to the user on a bad transform.

Thanks.

[ghost]

@evolross Just copy the code into your project. I had it in the file where I did all the CollectionFS stuff. The code is licensed under MIT license.

[evolross]

I got something working using your above handleTransformErrors function. Thank you! I did have to call handleError() to trigger the error versus just throw new Meteor.Error(). It wouldn't work right using the latter.

Since the error is ultimately set in copies.store.error one tricky thing I had to do on the client in order to check for the error was to create a Tracker.autorun after the file is inserted since in the callback of the File.insert in result, copies is not yet available. That only shows up on the client after it's uploaded and stored some moments later. So the autorun checks for Images.findOne(id).copies == null. Once that fails, I check for copies.store.error and stop the autorun. Not sure if this is the best way to do it, but it works.