Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crashing when trying to upload a folder #20

Merged
merged 2 commits into from
Nov 30, 2016
Merged

Fix crashing when trying to upload a folder #20

merged 2 commits into from
Nov 30, 2016

Conversation

thgeorgiou
Copy link
Contributor

Currently, if you try to upload a directory the tool crashes with Error: EISDIR: illegal operation on a directory, read and a stack trace. I added an error message to handle this case.

Maybe adding a 'recursive' flag to upload whole folders would be interesting? I could work on that 😄

Thanks for writing this tool by the way, very useful.

@AndiDittrich
Copy link
Owner

Dear Thanasis,

thanks for your contribution!

Your Pull-Request

  • The uploadFile::onComplete callback does not return any errors - it is called without an argument. Please do not alter the bevaiour - it should be consistent

Directory Uploads/Recursive

Instead of a recursive flag, i've projected to add support for the glob-pattern (https://www.npmjs.com/package/glob) - but it requires some larger refactorings

best regards, Andi

@thgeorgiou
Copy link
Contributor Author

The uploadFile::onComplete callback does not return any errors - it is called without an argument. Please do not alter the bevaiour - it should be consistent

I've updated the onComplete() callback, it should be consistent with the rest of the calls now.

@AndiDittrich AndiDittrich merged commit 709ecd7 into AndiDittrich:master Nov 30, 2016
AndiDittrich pushed a commit that referenced this pull request Nov 30, 2016
take care of the scope of fileInfo
@AndiDittrich
Copy link
Owner

thanks! i've moved the directory check into the try/catch block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants