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

Google Drive download_by_id does not have error handling #203

Open
2 tasks
augustuswm opened this issue Jun 24, 2022 · 0 comments
Open
2 tasks

Google Drive download_by_id does not have error handling #203

augustuswm opened this issue Jun 24, 2022 · 0 comments
Labels
needs fix Currently functionality that is broken

Comments

@augustuswm
Copy link
Contributor

augustuswm commented Jun 24, 2022

This is fundamentally an issue with our Google Drive client (and other clients), but tracking it here as it has direct impact.

When calling download_by_id, the Drive client will call to request_raw. request_raw will return the response body directly to the caller without can interference, and leaves it to the caller to perform any necessary error handling. download_by_id does not perform any checking of the response status or headers and instead immediately translates the request body into a bytes::Bytes. At this point, any data about the status of the request has been lost and a caller only has the raw bytes of the response to make determinations on.

In the case of cio, the bytes returned are being treated as a String, assuming that if the client had failed to download the file an error would have been returned. The visible results of this is the server error message being written to the database as if it was a successful download. Specifically we have seen this when failing to download a chat log file. This resulted in an error body for a 401 error overwriting the stored chat log data.

To resolve this we will need to address a couple sub-issues:

  • Update Google Drive client to return errors when download files fails
  • Only write chat log data when a file is successfully downloaded (currently unwrap_or_default is used which will overwrite with an empty string on failure)
@augustuswm augustuswm added the needs fix Currently functionality that is broken label Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs fix Currently functionality that is broken
Projects
None yet
Development

No branches or pull requests

1 participant