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

uploadFiles() doesn't return error status code or message in the promise catch()-method. #48

Closed
Crare opened this issue May 30, 2024 · 9 comments
Labels
P2 Important issue. Ready Ready for release.

Comments

@Crare
Copy link

Crare commented May 30, 2024

I have tried to handle file upload in the backend and return error in some business case validation, but I can't get the error result to show in the uploadFiles catch-method. It only gives same generic error everytime no matter what:

I've changed the url. but upload works, but error handling is non existent. I can't handle and show specific error to the user why their upload didn't work.

[Error: ENOENT: no such file or directory, open 'http://url/api/upload']

and on JSON.stringify:

err {
"nativeStackAndroid": [],
"userInfo": null,
"message": "ENOENT: no such file or directory, open 'http://url/api/upload'",
"code": "ENOENT"
}

import * as RNFS from '@dr.pogodin/react-native-fs';

// some other code for the component...

// inside the component on a call function:
RNFS.uploadFiles({
    toUrl: 'http://url.com/api/upload',
    files: files, // RNFS.UploadFileItemT[]
    method: 'POST',
    headers: {
        Accept: 'application/json',
        Authorization: `Bearer ${accessToken}`,
    },
    fields: {
        // form data
        Foo: 'Bar',
    }
})
    .promise.then(response => {
        console.log(response);
    })
    .catch(err => {
        console.log(err);
        console.log('err', JSON.stringify(err, null, ' '));
    })

// rest of the component...
@birdofpreyru birdofpreyru added the P2 Important issue. label May 30, 2024
@birdofpreyru
Copy link
Owner

Hmm... if look through the native code, uploadFiles() does have at least some code to catch and transmit actual errors; and if you search for the no such file or directory, open string in the codebase, you'll find it in multiple methods, but not in uploadFile() (perhaps uploadFile() depends on some of that methods that do throw this error). So... not sure what happens in your case, and how your toUrl value ends up in this error message (my first though would be a bug, but the example app has tests for uploadFile() that work fine).

It should be investigated, but uploadFiles() is not something I personally rely upon, thus a low priority for me, thus probably I won't look into it anytime soon.

@birdofpreyru birdofpreyru added the On Hold Blocked for some reason. label May 30, 2024
@Crare
Copy link
Author

Crare commented May 30, 2024

Thanks for looking into it. I did too. I found that in the onUploadComplete() row 757 in file ReactNativeFsModule.kt, it goes there in the reject()-method, which leads to rejectFileNotFound()-method and then to that message: "no such file or directory, open". I tried this by changing the values in every step to see where it leads. It would be useful if the error would return the statuscode all the way to the JS-code. But I checked that too, it looked like status code was 0. But should have been in my case 422 (Unprocessable Content), which is what our server sends in specific case we want to handle and a error-message comes there too from backend. This error happens after the files have been loaded to the server.

@birdofpreyru
Copy link
Owner

birdofpreyru commented May 30, 2024

Thanks for your donation @Crare , I'll mention you as a sponsor in the repo's README sometime later.

I believe the file/line you mention just propagates the error message, but does not generate it; and looking at the code, I guess, it is supposed that if it is just a server-side error there should be no exception there, it will just go the first branch of if-condition that do include statusCode into the result.

The actual error should be coming from here, meaning the upload() method there throws the exception:

try {
upload(mParams, res!!)
mParams!!.onUploadComplete?.onUploadComplete(res!!)
} catch (e: Exception) {
res!!.exception = e
mParams!!.onUploadComplete?.onUploadComplete(res!!)
}
I think, the easiest to debug will be if you just build & run your app in debug mode via Android Studio, set a breakpoint at the beginning of that upload() method, and go through it step-by-step, to see at what point it throws and why. The error reads like it fails when handling local file access, rather than because server responded with error code.

@Crare
Copy link
Author

Crare commented May 31, 2024

Hi! I found where the problem is finally by debugging that uploadFiles()-method in Android Studio. The problem is that the connection.inputStream is throwing exception and there is available connection.errorStream. Using the errorStream as responseStream I was able to get the error response from server to native code. And looks like it is now not going to reject(), as it is not adding the response to exception. but anyway it allows me to get the response result to JS-code which is helpful. I could add exception there when errorStream is not null at the end of Uploader.kt after it gets statusCode at the bottom of the upload()-method, but in that case the exception that is returned in promise.reject doesn't currently contain any statuscodes or error messages from server response.

Should I make a PR for this or do you have time to look into it? This was just a fix to Android side. I need to check iOS side too.

change:

if (connection.errorStream != null) {
    responseStream = BufferedInputStream(connection.errorStream)
} else {
    responseStream = BufferedInputStream(connection.inputStream)
}
responseStreamReader = BufferedReader(InputStreamReader(responseStream))

@Crare
Copy link
Author

Crare commented May 31, 2024

not sure if it should throw exception and go to promise.reject in that case if statusCode is not successful type or just check the result statuscode in the JS-code side.

the example handles the statusCode in the promise.resolve:


// upload files
RNFS.uploadFiles({
  toUrl: uploadUrl,
  files: files,
  method: 'POST',
  headers: {
    'Accept': 'application/json',
  },
  fields: {
    'hello': 'world',
  },
  begin: uploadBegin,
  progress: uploadProgress
}).promise.then((response) => {
    if (response.statusCode == 200) {
      console.log('FILES UPLOADED!'); // response.statusCode, response.headers, response.body
    } else {
      console.log('SERVER ERROR');
    }
  })
  .catch((err) => {
    if(err.description === "cancelled") {
      // cancelled by user
    }
    console.log(err);
  });

@birdofpreyru
Copy link
Owner

Ok, I see, I was wrong in my previous message, if server responds with an error status code the attempt to access input stream throws, as you say, and the library rejects the uploadFiles() promise.

Here are details from HttpURLConnection docs:
Screenshot from 2024-05-31 11-50-09

Great if your little patch resolves it for you, but for the library I guess it needs somewhat extra work and more complex solution: I guess, it should keep the past behavior and reject JS-side promise on such error, but it should indeed expose additional details from errorStream. So, perhaps on Native side it should do what you did, but then on JS side to check the status code in the resolved native promise, and depending on that to resolve or reject the caller-facing JS promise. That's my first impression, I should think about it more carefully. Then, tests for this updated behavior should be added to the example app, to ensure it works as intended on Android, and that it also works consistently for library user across Android/iOS/Windows; and lastly the README documentation should be updated to keep it clear how it works.

@birdofpreyru birdofpreyru removed the On Hold Blocked for some reason. label May 31, 2024
@birdofpreyru
Copy link
Owner

I don't quite have time to seriously look into all this right now, and presumably I'll be busy with higher priority stuff the next few weeks. I'd say, perhaps, use https://www.npmjs.com/package/patch-package to patch it directly in your project, copy/paste the patch into here, so I can refer to it whenever I have time, and then sometime later I probably look into adopting it into the library, perhaps with some modifications of the behavior, but keeping the actual functionality you need.

@Crare
Copy link
Author

Crare commented May 31, 2024

I checked iOS-side now, it already works in that way that it returns resolve with the statusCode and message in server error cases. So now they would be consistent with iOS and Android side. I don't know how to test the Windows-side.

@birdofpreyru
Copy link
Owner

Cool! Then it makes the life easier... still will need to ensure it is covered by tests in the example app, and correctly documented in README.

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

No branches or pull requests

2 participants