saket / telephoto

Building blocks for designing media experiences in Compose UI
https://saket.github.io/telephoto/
Apache License 2.0
1k stars 33 forks source link

Crash when caching is disabled by HTTP header `Cache-Control` #50

Closed haluzpav closed 4 months ago

haluzpav commented 1 year ago

My server is returning images with these HTTP headers.

Cache-Control: private, no-cache, no-store, must-revalidate
Expires: -1
Pragma: no-cache

This causes crash when used in Telephoto with Coil. The crash does not happen when using only Coil.

In my app (can't share), using me.saket.telephoto:zoomable-image-coil:0.5.0, the image shows up for the first time, and the crash occurs when the screen is re-opened and Telephoto attempts to access is from the cache.

FATAL EXCEPTION: main @coroutine#3232
Process: my.epic.app, PID: 11053
java.io.FileNotFoundException: No content provider: http://10.0.2.2:3000/my-fking-image
    at android.content.ContentResolver.openTypedAssetFileDescriptor(ContentResolver.java:2013)
    at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1842)
    at android.content.ContentResolver.openInputStream(ContentResolver.java:1518)
    at me.saket.telephoto.subsamplingimage.UriImageSource.peek(SubSamplingImageSource.kt:187)
    at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.isSvg(subSamplingEligibility.kt:56)
    at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.canBeSubSampled(subSamplingEligibility.kt:26)
    at me.saket.telephoto.zoomable.coil.Resolver.toSubSamplingImageSource(CoilImageSource.kt:153)
    at me.saket.telephoto.zoomable.coil.Resolver.work(CoilImageSource.kt:109)
    at me.saket.telephoto.zoomable.coil.Resolver$work$1.invokeSuspend(Unknown Source:14)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)

In the telephoto:sample app, at the latest commit, it crashes right away.

FATAL EXCEPTION: main
Process: me.saket.telephoto.sample, PID: 20902
java.io.FileNotFoundException: No content provider: http://10.0.2.2:3000/my-fking-image
    at android.content.ContentResolver.openTypedAssetFileDescriptor(ContentResolver.java:2013)
    at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1842)
    at android.content.ContentResolver.openInputStream(ContentResolver.java:1518)
    at me.saket.telephoto.subsamplingimage.UriImageSource.peek(SubSamplingImageSource.kt:187)
    at me.saket.telephoto.subsamplingimage.internal.ExifMetadata$Companion$read$2.invokeSuspend(ExifMetadata.kt:36)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@8104426, androidx.compose.runtime.BroadcastFrameClock@f14c867, StandaloneCoroutine{Cancelling}@792e614, AndroidUiDispatcher@31e3ebd]

Reproduction steps

  1. Let's mock a server somehow - using Mockoon, open this environment, change the body file to any image on your disk, and launch it.
  2. In emulator, the endpoint should be accessible at http://10.0.2.2:3000/my-fking-image
  3. In the telephoto project, apply these changes containg the URL and other stuff - patch file
  4. Launch the sample app and open the Breakfast item. Observe the crash.
  5. (bonus) If you uncomment respectCacheHeaders(false), the app no longer crashes, but it gets stuck in cycle, observe "Start" being printed over and over in the log, image never showing up.

Expected: App works with "no-cache" HTTP headers, not attempting to save to cache, either memory or disk, and not crashing. Furthermore, respectCacheHeaders(false) should allow me to use cache, see.

haluzpav commented 1 year ago

More weirdness - the bonus point 5. is working fine when I remove the listener I've added for debugging. 🤔 The app also crashes only the second time I open the larger image, same as in my app.

janbina commented 1 year ago

We are encountering the same crash in our app, however, we are not able to reproduce it. It happened to me once with url that worked before and after just fine, it seems quite random. We can also see quite a bit of such crashes in crashlytics, this is one of them:

java.io.FileNotFoundException: No content provider: https://d34-a.sdn.cz/d_34/c_img_gT_q/dMUUDV.jpeg?fl=res,2100,2000,1
       at android.content.ContentResolver.openTypedAssetFileDescriptor(ContentResolver.java:1996)
       at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1825)
       at android.content.ContentResolver.openInputStream(ContentResolver.java:1502)
       at me.saket.telephoto.subsamplingimage.UriImageSource.peek(SubSamplingImageSource.kt:187)
       at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.isSvg(subSamplingEligibility.kt:56)
       at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.canBeSubSampled(subSamplingEligibility.kt:26)
       at me.saket.telephoto.zoomable.coil.Resolver.toSubSamplingImageSource(CoilImageSource.kt:153)
       at me.saket.telephoto.zoomable.coil.Resolver.work(CoilImageSource.kt:109)
       at me.saket.telephoto.zoomable.coil.Resolver$work$1.invokeSuspend(CoilImageSource.kt)

You can check the image, it's a real url: https://d34-a.sdn.cz/d_34/c_img_gT_q/dMUUDV.jpeg?fl=res,2100,2000,1

We've been using version 0.5.0, can't confirm it also happens with newer versions as we are not able to reproduce it and we didn't publish app with newer version yet.

Hospes commented 6 months ago

I'm having the same crash reporting on version 0.11.2. But I can't confirm it happens because of no cashe in headers. We have this header:

Cache-Control:   public, max-age=31919000

Crash report:

      Fatal Exception: java.io.FileNotFoundException: No content provider: https://images.whoppah.com/products/L17DJ822JFEN/P620HF9ZEVQP/vintage-bookcase.jpeg?width=1920
       at android.content.ContentResolver.openTypedAssetFileDescriptor(ContentResolver.java:2020)
       at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1849)
       at android.content.ContentResolver.openInputStream(ContentResolver.java:1525)
       at me.saket.telephoto.subsamplingimage.UriImageSource.peek(SubSamplingImageSource.kt:190)
       at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.canBeSubSampled(subSamplingEligibility.kt:47)
       at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt.access$canBeSubSampled(subSamplingEligibility.kt:1)
       at me.saket.telephoto.zoomable.coil.SubSamplingEligibilityKt$canBeSubSampled$2.invokeSuspend(subSamplingEligibility.kt:27)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
       at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
       at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:111)
       at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:99)
       at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:811)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:715)
       at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:702)
saket commented 4 months ago

Apologies for taking so long to address this. There were actually three different issues that @haluzpav was facing:

  1. The image URL was incorrectly being parsed as a content URI.

  2. ZoomableAsyncImage() wasn't ignoring the no-store cache HTTP headers. This was preventing sub-sampling and kinda defeating the purpose of using telephoto.

  3. Creating a new request listener on every composition was causing an inifinite loop of image reloads.

Issues 1 and 2 have been fixed. Telephoto will now ignore no-store HTTP headers to force sub-sampling of large images. This is admittedly a hidden behavior, but I'm going ahead with this for now.

Issue 3 is hard to fix, but telephoto will now detect these infinite loops and throw a helpful error message so that you can identify and fix the problem. I'll add some documentation to the project website in the next release, but in the meantime, you need to remember your request listener:

  ZoomableAsyncImage(
    model = ImageRequest.Builder(LocalContext.current)
      .data("https://example.com/image.jpg")
-     .listener(onStart = { … })
+     .listener(
+      remember {
+         object : ImageRequest.Listener {
+           override fun onStart(…) {}
+         }
+       }    
+     )
      .crossfade(1_000)
      .memoryCachePolicy(CachePolicy.DISABLED)
      .build(),
    contentDescription = …
  )

These fixes will be released in 0.12.0.

saket commented 4 months ago

Issue 3 is hard to fix

@colinrtwhite has proved me wrong by pointing out Coil's EqualityDelegate. I was able to easily integrate it into Telephoto. You'll no longer need to remember your request listeners and other details in the next release.

saket commented 4 months ago

Fixed in 0.12.0: https://github.com/saket/telephoto/releases/tag/0.12.0