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

Deprecation warning on newer versions of Grunt #12

Closed
MSnoeren opened this issue Oct 5, 2016 · 16 comments
Closed

Deprecation warning on newer versions of Grunt #12

MSnoeren opened this issue Oct 5, 2016 · 16 comments
Assignees

Comments

@MSnoeren
Copy link

MSnoeren commented Oct 5, 2016

When using HTTP Upload together with Grunt version 1.0.0 or higher a deprecation warning is raised during the upload; "fs.read's legacy String interface is deprecated. Use the Buffer API as mentioned in the documentation instead."

It does succesfully upload and work. Any way to fix this message?

@julienma
Copy link
Member

julienma commented Oct 5, 2016

This seems related to Node v6.0.0, see release notes, and this PR in particular.

I guess we should upgrade dependencies on this package, we're still on node 0.8.0.
If you're willing to do a PR that'd be great; otherwise I'll try to work on that in the coming weeks.

@julienma julienma self-assigned this Oct 5, 2016
@julienma
Copy link
Member

Ok, so this is indeed related to a deprecation warning introduced in Node v6.0.0, and actually concerns Restler, a dependency used for multipart upload. I think this is the culprit line on Restler's side: https://github.com/danwrong/restler/blob/master/lib/multipartform.js#L118

I'll see if Restler can be updated (but likely not, judging from its activity), or if there is another library available (I found https://github.com/aacerox/node-rest-client, but am not sure it handles multipart uploads).
In the meanwhile, it doesn't affect grunt-http-upload, as uploads work fine.

@julienma
Copy link
Member

@MSnoeren1995 I've fixed this by replacing a dependency (replace Restler with Request).

It's not published yet to NPM.
Please confirm it's working as expected by updating grunt-http-upload:

npm install DiscoverGrunt/grunt-http-upload.git#v0.3.0 --save-dev

Then run your grunt task.
You should not have anymore the deprecation warning, and everything else should work as it used to be.

Thanks for your feedback.

@julienma
Copy link
Member

Your packages depend on http-grunt-upload.

I'll soon publish v0.3.0 on NPM, which contain a major, if not breaking, change.
If you could test the new version before it is pushed to NPM (see my comment above), it'd be great. You might catch some unexpected errors.

Thanks.

@MSnoeren
Copy link
Author

I've tested the new version and can confirm that it works with the Request dependency. The deprecated message is gone now. The only thing that changed is that it does not handle redirects well, but that shouldn't be much of an issue. Thank you!

@piotrbogun
Copy link

I just tested using the new version without any issues. Will let you know if I do encounter anything. Thanks for notifying us @julienma !

@julienma
Copy link
Member

Thanks both for your feedback.
@MSnoeren1995 could you give me more info on your redirect issue? https://github.com/request/request explains that it follows redirects (up to 10 with default settings).

@MSnoeren
Copy link
Author

I'm using your software to push a zip-file to a Joomla! site and install it directly. Before, I could fill in http://host.com/upload while it was redirected to https://host.com/upload?format=raw without problems; now it gives me HTTP 301 and 302 errors, which are common HTTP errors used for redirection. I have to put in the full and last URL. Then it works as it should be.

@julienma
Copy link
Member

I guess you're using POST/PUT for that, not GET?

@MSnoeren
Copy link
Author

Yes, I use a POST.

@julienma
Copy link
Member

Ok, I think I know why. I'll post an update tonight.

@julienma
Copy link
Member

@MSnoeren1995 try again with latest release, it should fix the redirect issue:

npm install DiscoverGrunt/grunt-http-upload.git#v0.3.1 --save-dev

It it works fine, I'll release on NPM.

@MSnoeren
Copy link
Author

Strangely enough, it now gives an 500 error where a wget without redirects gives me a 302 Found error. It doesn't seem to come from my server, though. Any idea what causes this? Using the right url that does not redirect does still work.

@julienma
Copy link
Member

Strange. I'm not sure what to do about this, my HTTP skills don't allow me to understand what's going on.
Googling around a bit, it seems that following redirects for methods other than GET is not a good practice. I'll just deactivate my last patch and leave it as yesterday night (v.0.3.0).
You'll have to give the exact URL, but that should be fine enough, right?

@julienma
Copy link
Member

Ok, I've reverted that last bit about redirections, and have published latest version (v0.3.2) on NPM.
Thanks all for your feedback!

@MSnoeren
Copy link
Author

I know that redirecting POST request isn't recommended, but strangely it worked. I'm using http_upload on alot of projects and have to edit them all, but that is not an issue. Thank you :)

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

No branches or pull requests

3 participants