-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
fix: crash of file system when try to read cache dir file on android #13232
fix: crash of file system when try to read cache dir file on android #13232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review previously left here is no longer valid, jump to the latest one 👉 #13232 (review)
85e5d27
to
67fe814
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review previously left here is no longer valid, jump to the latest one 👉 #13232 (review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 🚀
Please add a changelog entry and add a comment in the code why we need to strip the file
scheme.
packages/expo-file-system/android/src/main/java/expo/modules/filesystem/FileSystemModule.java
Outdated
Show resolved
Hide resolved
67fe814
to
82d02b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review previously left here is no longer valid, jump to the latest one 👉 #13232 (review)
82d02b9
to
128e1ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review previously left here is no longer valid, jump to the latest one 👉 #13232 (review)
@lukmccall i have done requested changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.
Looks like I have nothing to complain about 👏 Keep up the good work! 💪
Generated by ExpoBot 🤖 against fef6804
fixed #5408
Why
Should resolve to
{ exists: false }
instead of throwingHow
getInfoAsync
throws an errorFile <file path isn't readable>
Steps to Reproduce
try to read file from
cacheDirectory
it will throw an exception which this PR has fixedChecklist
expo build
(eg: updated@expo/xdl
).expo prebuild
& EAS Build (eg: updated a module plugin).Reproducible Demo
https://snack.expo.io/@charliecruzan/getinfoasyncerror