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

File encoding with drive API broken #1151

Closed
JaakkoL opened this issue May 7, 2018 · 4 comments
Closed

File encoding with drive API broken #1151

JaakkoL opened this issue May 7, 2018 · 4 comments
Assignees
Labels
external This issue is blocked on a bug with the actual product. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@JaakkoL
Copy link

JaakkoL commented May 7, 2018

Google Drive API allows specifying file encoding for the response data. Underneath it's overriding Axios options, like the documentation states.

However, with the current latest Axios release (0.18.0), which this library is using, this is not implemented. This axios/axios#869 PR has that feature implemented, but it's not part of the released package. Also, it seems that even with these changes included, the Google Node Client implementation regarding file encoding doesn't work in the documented way (how it used to work prior to the switch to Axios from Request).

@JustinBeckwith
Copy link
Contributor

Greetings! Can you share the exact API you're trying to call? Or the docs that call out you can specify the file encoding? Just want to make sure I'm reading the right thing :)

@JustinBeckwith JustinBeckwith added needs more info This issue needs more information from the customer to proceed. external This issue is blocked on a bug with the actual product. labels May 10, 2018
@JaakkoL
Copy link
Author

JaakkoL commented May 11, 2018

Sure thing @JustinBeckwith !

So basically what were using is Google Drive API for downloading a single file.

const { data } = await drive.files.get({ fileId: '<redacted>', alt: 'media' }, { encoding: 'latin1' });

In the documentation on request level options of the client, it says:

You can also override axios options per request, such as url, method, and encoding.

Passing the encoding in request options used to work at least before googleapis switched from request to axios.

I digged into axios code and found out some issues on their current release, which is not even supporting the encoding options (which is btw responseEncoding instead of encoding), but they're working on to resolve this. See axios/axios#1519

So I think that in order to resolve this, and continue supporting response encoding option, axios needs to first resolve issues on their side and after that probably updating documentation and examples to the correct request level options format (responseEncoding etc.) should be enough.

In the meanwhile, this can be resolved (at least on our end) in the application logic by returning array buffer from the request and handling the encoding in the application logic itself instead relying on the library. E.g.

const { data } = await drive.files.get({ fileId: '<redacted>', alt: 'media' }, { responseType: 'arraybuffer' });
data.toString('latin1')

@JustinBeckwith JustinBeckwith added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed needs more info This issue needs more information from the customer to proceed. labels May 28, 2018
@JustinBeckwith JustinBeckwith added the 🚨 This issue needs some love. label Nov 3, 2018
@JustinBeckwith JustinBeckwith removed the 🚨 This issue needs some love. label Jul 10, 2019
@JustinBeckwith
Copy link
Contributor

Since we last took a look here, we switched our underlying transport layer from axios to our own client, gaxios. I suspect the problem is solved, but wanted to check in :)

Are you still having issues here?

@JaakkoL
Copy link
Author

JaakkoL commented Dec 8, 2019

I think this can be considered resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external This issue is blocked on a bug with the actual product. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants