Skip to content
This repository has been archived by the owner. It is now read-only.

[1.0.0] Add better error trace on transform streams #227

Open
raix opened this Issue Mar 27, 2014 · 8 comments

Comments

Projects
None yet
7 participants
@raix
Copy link
Contributor

raix commented Mar 27, 2014

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

@raix raix added this to the 1.0.0 CollectionFS core milestone Mar 27, 2014

@Sanjo

This comment has been minimized.

Copy link
Contributor

Sanjo commented May 12, 2014

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. (#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

This comment has been minimized.

Copy link
Member

aldeed commented May 12, 2014

@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

This comment has been minimized.

Copy link

piyushcoader commented Aug 24, 2014

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

This comment has been minimized.

Copy link

cristiandley commented Oct 3, 2014

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

This comment has been minimized.

Copy link

RobertLowe commented Jul 9, 2015

+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

This comment has been minimized.

Copy link

evolross commented Aug 14, 2015

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.

@Sanjo

This comment has been minimized.

Copy link
Contributor

Sanjo commented Aug 16, 2015

@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

This comment has been minimized.

Copy link

evolross commented Sep 3, 2015

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.