alpha0010 / react-native-file-access

Filesystem access for React Native
MIT License
311 stars 23 forks source link

Failed Fetches with an `ok` of false still creates a file but corrupted #87

Closed itsramiel closed 1 month ago

itsramiel commented 2 months ago

react-native: 0.74.5 react-native-file-access: 3.1.0 Platform: both

Bug When I fetch an asset with FileSystem.fetch and the request fails for some reason, such as 403, 404, then the asset is still downloaded and saved

To reproduce Try out this sample that download an image with unanavailable url:

import * as RNFA from 'react-native-file-access';
import {Button, StyleSheet, View} from 'react-native';

const url =
  'https://buffer.com/library/content/images/size/w1200/2023/10/freimages.jpg';

function App(): React.JSX.Element {
  return (
    <View style={styles.screen}>
      <Button
        title="press me"
        onPress={async () => {
          const path = `${RNFA.Dirs.DocumentDir}/test.png`;
          console.log('path', path);
          const res = await RNFA.FileSystem.fetch(url, {path: path});
          console.log('res', res);
          const exists = await RNFA.FileSystem.exists(path);
          console.log('exists', exists);
        }}
      />
    </View>
  );
}

const styles = StyleSheet.create({
  screen: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
});

export default App;

You will notice that the response is 404 not found but the file is still created with the path. Why would a failed fetch create a file?

alpha0010 commented 1 month ago

The reason for still creating the file is with, for example, a JSON api, the returned blob on 404 (or other 4xx and 5xx errors) may still have relevant information (like an error message to display in app). I cannot think of a way to automatically determine if this library should discard completed server responses, even when not 2xx OK replies.

itsramiel commented 1 month ago

doesnt it make more sense to return the response as an error if it fails or some other way?

alpha0010 commented 1 month ago

The thing is, in the context of a fetch request, 404 (and other non-200 replies) is not a failure for the fetch. The library successfully contacted the target server, got a response, and saved it. The content of the response might be an error message generated by the server, but that is application level logic, not library level logic. A 404 response is fully valid and expected, if querying an api for the existence of a resource.

If network is down, connection times out, dns fails to resolve, etc., fetch will produce an error, since it failed to get a response from the server.

itsramiel commented 1 month ago

oh alright got it