adamhenson / s3-image-uploader

A Node.js module for resizing, and uploading files to Amazon S3 with capability to track progress using websockets.
Other
44 stars 5 forks source link

There was a problem uploading this file. #2

Closed playground closed 7 years ago

playground commented 7 years ago

Is there a way to get better error message? All I'm getting is "There was a problem uploading this file".

adamhenson commented 7 years ago

Thanks @playground - I agree. You must be seeing this message in the errorCallback of Uploader.upload.

Originally, I don't think I wanted to expose server information, so I created a generic message. However, I think details are necessary here.

Therefore I provided an additional parameter to errorCallback. I updated the README, but here's an example of how one could use it:

uploader.upload({
  fileId : 'someUniqueIdentifier',
  bucket : 'somebucket',
  source : './public/tmp/myoldimage.jpg',
  name : 'mynewimage.jpg'
},
function(data){ // success
  console.log('upload success:', data);
  // execute success code
},
function(errMsg, errObject){ //error
  console.error('unable to upload: ' + errMsg + ':', errObject);
  // execute error code
});

You'll need to re-install this module to test. I've updated in NPM.

playground commented 7 years ago

Thanks @adamhenson for the quick response. That error message comes in handy :-). I was able to upload the image to s3 bucket.

adamhenson commented 7 years ago

Excellent, glad this helped @playground. Thanks for calling it out.

On Wed, Nov 9, 2016 at 9:48 AM, playground notifications@github.com wrote:

Thanks @adamhenson https://github.com/adamhenson for the quick response. That error message comes in handy :-). I was able to upload the image to s3 bucket.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adamhenson/s3-image-uploader/issues/2#issuecomment-259431446, or mute the thread https://github.com/notifications/unsubscribe-auth/AAto34sSZ7C5kRI_wBTkefWbrYA9b-MHks5q8d0ogaJpZM4KsrFS .

doublesharp commented 7 years ago

This makes it more difficult to use Promises unfortunately.

I realize this would be a breaking change, but using the pattern uploader.upload(options, callback(err, data)) would allow the modules to be promisified via es6-promisify.

On success you would return callback(null, data) and on error you would return(err, null) which would translate to

const promisify = require('es6-promisify');

promisify(uploader.upload)(options)
.then(data => console.log(data))
.catch(err => console.log(err));
adamhenson commented 7 years ago

@doublesharp - I'm not familiar with es6-promisify, and I'm not sure if I follow your point. errObject is an optional argument. I believe your example above would not be affected by this change.

In .catch(err => console.log(err)); - wouldn't err represent errMsg from my example above?

doublesharp commented 7 years ago

I was referring to "node style callbacks" or "error first".

http://thenodeway.io/posts/understanding-error-first-callbacks/

On Dec 8, 2016 1:34 PM, "Adam" notifications@github.com wrote:

@doublesharp https://github.com/doublesharp - I'm not familiar with es6-promisify, and I'm not sure if I follow your point. errObject is an optional argument. I believe your example above would not be affected by this change.

In .catch(err => console.log(err)); - wouldn't err represent errMsg from my example above?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adamhenson/s3-image-uploader/issues/2#issuecomment-265862188, or mute the thread https://github.com/notifications/unsubscribe-auth/AAi4UFBqLxcWv8f2HyE368V_3r4mYy33ks5rGHfMgaJpZM4KsrFS .

adamhenson commented 7 years ago

Right, @doublesharp. I really haven't given this project much attention for some time. It could use some refactoring to support promises.

Your concerns are valid, but irrelevant to this issue. It's more relevant to this project as a whole, as the issue you describe existed before this one (#2).

Feel free to submit your issue... or even better a PR to address your concerns.

doublesharp commented 7 years ago

It would definitely affect the project as a whole, so it would probably be more appropriate for a major version update if you did it at all - no pressure at all. I was looking into error handling which is why I thought this was an appropriate place to comment. I'll certainly submit a PR if I make any updates. Thanks!

adamhenson commented 7 years ago

Certainly, your comments are welcome. Thanks @doublesharp. Hopefully sometime soon I can give this project more love.