Task/upload files crash fix
Description
This PR fixes file uploading crash. I have tried to reproduce this crash but I was unsuccessful after numerous attempts. Although after going through the logs, the crash was a permission error specifically for WRITE_EXTERNAL_STORAGE and INTERNET permissions. The WRITE_EXTERNAL_STORAGE was causing the FileNotFoundException and INTERNET permissions caused a connection error leading to a StorageFileLoadException.
The PR adds checks to confirm that the permissions have been enabled and request permissions if they are not enabled.
Fixes #2285
Testing Instructions
Screenshots or Screencast
- Sign in
- Go to profile
- Click files
- Add a new file
Checklist
- [ ] If this is a user-facing change, I have added an entry in CHANGELOG.md
- [X] Ensure the linter passes (
./gradlew spotlessApplyto automatically apply formatting/linting) - [ ] I have considered whether it makes sense to add tests for my changes
- [ ] All strings that need to be localized are in
modules/services/localization/src/main/res/values/strings.xml - [ ] Any jetpack compose components I added or changed are covered by compose previews
- [ ] I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.
I have tested any UI changes...
- [ ] with different themes
- [ ] with a landscape orientation
- [ ] with the device set to have a large display and font size
- [ ] for accessibility with TalkBack
I'm sorry but the reasoning doesn't explain the #2285 issue. The stack trace shows that the crash happens for an input stream which means reading a file and not writing to one.
WRITE_EXTERNAL_STORAGE is needed for writing to an external shared storage. And since SDK 33 this permission is deprecated and instead file interactions should be handled via createWriteRequest(). This is something we should probably look into as a separate issue.
INTERNET permission doesn't make sense because it is a normal permission granted at the installation time.
I'm sorry but the reasoning doesn't explain the #2285 issue. The stack trace shows that the crash happens for an input stream which means reading a file and not writing to one.
WRITE_EXTERNAL_STORAGEis needed for writing to an external shared storage. And since SDK 33 this permission is deprecated and instead file interactions should be handled via createWriteRequest(). This is something we should probably look into as a separate issue.
INTERNETpermission doesn't make sense because it is a normal permission granted at the installation time.
Thanks for the feedback, so we should check permissions for READ_EXTERNAL_STORAGE
I will go ahead and remove the INTERNET permissions check. The reasoning to that was since we also had a connection error, we should have a check for the permission, but since its a normal permission we don't need that.
@MiSikora any update on this?
The solution isn't correct. You're invoking the permission request after the file request has been submitted. Also, this permission is deprecated in SDK 33+ and doesn't work there. And most importantly this permission shouldn't be needed since we're not accessing other apps' files when using android.intent.action.GET_CONTENT Intent.
If you want to fix this issue I'm going to have to ask you to provide a reproducible scenario that is fixed with your changes.
I'm closing this PR as there is no reproducible scenario that we could test this with and the proposed solution has issues that I mentioned above.