Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

downloadFile should not return when statusCode is not in 2xx range #37

Closed
kevinboosten opened this issue Jul 21, 2020 · 4 comments · Fixed by #38
Closed

downloadFile should not return when statusCode is not in 2xx range #37

kevinboosten opened this issue Jul 21, 2020 · 4 comments · Fixed by #38

Comments

@kevinboosten
Copy link

Describe the bug
Using the downloadFile method successfully resolves even when the http statuscode is not in the 2xx range.

To Reproduce
Steps to reproduce the behavior:

  1. run the following code:
try {
   const result = await Http.downloadFile({
     url: 'https://bogus.com/myimage.png,
     filePath: 'myfilename.png',
     fileDirectory: FilesystemDirectory.Cache
   });
   console.log('download result: ', result);
} catch (error) {
   console.error('download error: ', error);
}

Expected behavior
I would expect the method to fail since the http statuscode in the example is equal to: 404.
Any statuscode that is not in the range of 2xx should reject the downloadFile method invocation.

On ios and web the downloadFile method will resolve successfully. On android you will end up in your catch block (like expected).
So there's a mismatch in Capacitor http api depending on the platform that should be handled by this plugin.

Screenshots
N/A

Additional context

"@capacitor-community/http": "^0.2.1
kevinboosten pushed a commit to kevinboosten/http that referenced this issue Jul 21, 2020
Any statuscode that is not in the range of 2xx should reject the downloadFile method invocation.

capacitor-community#37
@kevinboosten
Copy link
Author

kevinboosten commented Jul 21, 2020

You could also argue about if the following code should result in an error:

await Http.request({
   url: 'http://httpstat.us/404',
   method: 'GET'
});

I would expect this (if I compare it to the Angular HttpClient):
Http.request --> statuscode in 2xx range --> ✅
Http.request --> statuscode not in 2xx range --> ❌

This way people can, perhaps, more easily use this api without specifically having to interpret the statusCode. But I guess this has been implemented this way deliberately, right?

@imhoffd
Copy link
Contributor

imhoffd commented Jul 21, 2020

I believe it was modeled after fetch behavior, which only rejects if there's an issue with the connection. That being said, the behavior should be the same across all platforms.

@kevinboosten
Copy link
Author

I believe it was modeled after fetch behavior, which only rejects if there's an issue with the connection. That being said, the behavior should be the same across all platforms.

For the "Http.request" part I can agree. The downloadFile method should reject or we should add the statuscode in the response so the user can decide what to do with it. But then the method should not create a file on filesystem.

@imhoffd
Copy link
Contributor

imhoffd commented Jul 21, 2020

Agreed, downloadFile is a higher level method.

thomasvidas pushed a commit that referenced this issue May 18, 2021
Any statuscode that is not in the range of 2xx should reject the downloadFile method invocation.

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

Successfully merging a pull request may close this issue.

2 participants