Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

IOException when app uses 3rd party SDK #422

Closed
vbuberen opened this issue Aug 13, 2020 · 8 comments · Fixed by #427
Closed

IOException when app uses 3rd party SDK #422

vbuberen opened this issue Aug 13, 2020 · 8 comments · Fixed by #427
Labels
bug Something isn't working
Milestone

Comments

@vbuberen
Copy link
Collaborator

✍️ Describe the bug

This issue arises from #394

After fixes for #394 the reporter provided update that another exception appeared:

 java.io.IOException: No such file or directory
        at java.io.UnixFileSystem.createFileExclusively0(Native Method)
        at java.io.UnixFileSystem.createFileExclusively(UnixFileSystem.java:281)
        at java.io.File.createNewFile(File.java:1008)
        at com.chuckerteam.chucker.internal.support.AndroidCacheFileFactory.create(AndroidCacheFileFactory.kt:21)
        at com.chuckerteam.chucker.internal.support.AndroidCacheFileFactory.create(AndroidCacheFileFactory.kt:15)
        at com.chuckerteam.chucker.api.ChuckerInterceptor.multiCastResponseBody(ChuckerInterceptor.kt:184)
        at com.chuckerteam.chucker.api.ChuckerInterceptor.intercept(ChuckerInterceptor.kt:92)

It happens even after removing the application from device and reinstalling a new one.

🔧 Expected behavior

Chucker works normally and shows transactions.

📱 Tech info

  • Chucker version: 3.3.0-SNAPSHOT (as of 11th of August 2020)

📄 Additional context

We should investigate the issue to either fix it or to get all the info we might need to deal with possible problems with other 3rd party SDKs.

@vbuberen vbuberen added bug Something isn't working incomplete This issue needs more data in order to be triaged labels Aug 13, 2020
@vbuberen
Copy link
Collaborator Author

@CodeBreak524 Could you provide us with some more info? What request occurs at the point where you see this exception?
Since the SDK mentioned in your issue is closed one we need some bread crumbs to try and understand what happens there.

@koral--
Copy link
Contributor

koral-- commented Aug 13, 2020

From what I can see this issue can be easily reproduced. Just delete the cache directory when app is running.
No matter what is the exact reason the solution is to make sure that parent directory exists before calling File#createNewFile().
It can be done by just calling parentFile.mkdirs() before this line:


or by invoking Context#getCacheDir() on each file creation, not only at the init time:
Note that cacheDir may be null which is also unhandled by Chucker and reported in #414.

@CodeBreak524
Copy link

It seems that behaviour described above by @koral-- is the same for my case.

It always happens after giving control to IdNow SDK and it looks like it removes all cache files and directories of app while processing. Of course it is not correct behaviour in IdNow but I also suppose that Chucker should handle such cases without throwing an error for request

@MiSikora
Copy link
Contributor

MiSikora commented Aug 13, 2020

I'm not sure if this can be reproduced this way. We have tests that should cover mentioned behaviour, and they are result of the error reported in #394. Unless this does not reproduce that or JVM behaves differently than Android when it comes to exceptions.

@ParameterizedTest
@EnumSource(value = ClientFactory::class)
fun responseReplicationFailure_doesNotAffect_readingByConsumer(factory: ClientFactory, @TempDir tempDir: File) {
assertThat(tempDir.deleteRecursively()).isTrue()
val body = Buffer().apply { writeUtf8("Hello, world!") }
server.enqueue(MockResponse().setBody(body))
val request = Request.Builder().url(serverUrl).build()
val chuckerInterceptor = ChuckerInterceptorDelegate(TestFileFactory(tempDir))
val client = factory.create(chuckerInterceptor)
val responseBody = client.newCall(request).execute().body()!!.source().readByteString()
assertThat(responseBody.utf8()).isEqualTo("Hello, world!")
}
@ParameterizedTest
@EnumSource(value = ClientFactory::class)
fun responseReplicationFailure_doesNotInvalidate_ChuckerTransaction(factory: ClientFactory, @TempDir tempDir: File) {
assertThat(tempDir.deleteRecursively()).isTrue()
val body = Buffer().apply { writeUtf8("Hello, world!") }
server.enqueue(MockResponse().setBody(body))
val request = Request.Builder().url(serverUrl).build()
val chuckerInterceptor = ChuckerInterceptorDelegate(TestFileFactory(tempDir))
val client = factory.create(chuckerInterceptor)
client.newCall(request).execute().readByteStringBody()
val transaction = chuckerInterceptor.expectTransaction()
assertThat(transaction.responseBody).isNull()
assertThat(transaction.responsePayloadSize).isEqualTo(body.size())
}

@koral--
Copy link
Contributor

koral-- commented Aug 13, 2020

I've added a test demonstrating this scenario. #423

@MiSikora
Copy link
Contributor

MiSikora commented Aug 13, 2020

Ok, so the fix would be to return File? from FileFactory and handle IO and possible NPE.

From the design perspective, it might be good to open the possibility of passing the temp files directory in the constructor. This way users could provide more reliable directory from their perspective. The issue I see is that it would make usage of files public, which would require major version bump to remove from the constructor. Also, file usage might be too new to stick to it right now. @vbuberen @cortinico what do you think?

@koral--
Copy link
Contributor

koral-- commented Aug 14, 2020

I'm not sure if that is not an overengineering.

IMHO the sufficient fix for this issue is to not save context.getCacheDir() result in a property but invoke it each time just before attempt to create the file. This should solve the vast majority of issues related to this ticket (getCacheDir() tries to create a directory if it does not exist).

Additionally we can add nullcheck and probably throw some IOException with appropriate message.

If I understand correctly IOException here is now handled gracefully and does not cause request to be dropped.

@ghost ghost added the Pending PR The resolution for the issue is in PR label Aug 17, 2020
cortinico pushed a commit that referenced this issue Aug 22, 2020
koral-- added a commit to DroidsOnRoids/chucker that referenced this issue Aug 23, 2020
@ghost ghost removed Pending PR The resolution for the issue is in PR bug Something isn't working incomplete This issue needs more data in order to be triaged labels Aug 25, 2020
@CodeBreak524
Copy link

Thanks, guys. Now it seems to be working without requests failing.

Special thanks to @koral-- for providing cases and scenarios to reproduce the problem

@vbuberen vbuberen added this to the 3.3.0 milestone Sep 29, 2020
@vbuberen vbuberen added the bug Something isn't working label Sep 29, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants