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.

Sign up for GitHub

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

Jump to bottom

Missing error handling for Errors #435

Closed
TWiStErRob opened this issue Apr 22, 2015 · 2 comments
Closed

Missing error handling for Errors #435

TWiStErRob opened this issue Apr 22, 2015 · 2 comments
Labels
bug

Comments

@TWiStErRob
Copy link
Collaborator

Glide Version/Integration library (if any): 3.6.0-SNAPSHOT / okhttp-1.3.0-SNAPSHOT
Device/Android Version: S4/4.4
Issue details: Runnables running in pools or just Threads don't have error handling and silently fail. This means that EngineRunnable.run is unguarded. There's no exception logged and even the the internal EngineRunnable.onLoadFailed is missing a call, so the .error() drawable is not set, nor any result. It was really weird to debug the code, because the only thing I experienced is that fetcher.loadData in DecodeJob.decodeSource throws something (I know this because it jumped to finally), but there was no way to find out what.

Over the years I learned to writing a Runnable by starting with try {} catch(Throwable t) {}, because otherwise I'll never be notified of the problems I may have during development, neither it is healthy to let those escape scope and become unreported. FifoPriorityThreadPoolExecutor also just calls run so the only way to catch these is escaped RuntimeExceptions/Errors is Thread.set(Default)UncaughtExceptionHandler. I think the library should catch those in one way or another and report them through the usual paths.

Repro steps: Add the following dependencies to the project and run any http image load:

  • glide-3.6.0-20150421.213351-34.jar
  • okhttp-integration-1.3.0-20150421.213406-33.jar (depends on glide and okhttp)
  • okhttp-2.3.0.jar (depends on okio)
    The important thing is to NOT add okio-1.3.0.jar to the classpath, or just exclude it (untested, I was using the libs folder):
compile('com.squareup.okhttp:okhttp:2.3.0') {
    exclude group: 'com.squareup.okio', module: 'okio'
}

Suggested fix: the only way I was able to figure out what's happening is by adding:

} catch(Error e) {
    Log.w("XXX", "Cannot decode", e);
}

to EngineRunnable.run. The onException interface and others can't handle Throwables so the Errors may need to be wrapped in UncaughtErrorException or similar, or just caught and logged.

Stack trace:
This was what I got when I modified the Glide sources to log:

java.lang.NoClassDefFoundError: okio.Okio
            at com.squareup.okhttp.internal.http.HttpConnection.<init>(HttpConnection.java:89)
            at com.squareup.okhttp.Connection.connect(Connection.java:161)
            at com.squareup.okhttp.Connection.connectAndSetOwner(Connection.java:175)
            at com.squareup.okhttp.OkHttpClient$1.connectAndSetOwner(OkHttpClient.java:120)
            at com.squareup.okhttp.internal.http.HttpEngine.nextConnection(HttpEngine.java:330)
            at com.squareup.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:319)
            at com.squareup.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:241)
            at com.squareup.okhttp.Call.getResponse(Call.java:271)
            at com.squareup.okhttp.Call$ApplicationInterceptorChain.proceed(Call.java:228)
            at com.squareup.okhttp.Call.getResponseWithInterceptorChain(Call.java:199)
            at com.squareup.okhttp.Call.execute(Call.java:79)
            at com.bumptech.glide.integration.okhttp.OkHttpStreamFetcher.loadData(OkHttpStreamFetcher.java:40)
            at com.bumptech.glide.integration.okhttp.OkHttpStreamFetcher.loadData(OkHttpStreamFetcher.java:19)
            at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.loadData(ImageVideoModelLoader.java:70)
            at com.bumptech.glide.load.model.ImageVideoModelLoader$ImageVideoFetcher.loadData(ImageVideoModelLoader.java:53)
            at com.bumptech.glide.load.engine.DecodeJob.decodeSource(DecodeJob.java:170)
            at com.bumptech.glide.load.engine.DecodeJob.decodeFromSource(DecodeJob.java:128)
            at com.bumptech.glide.load.engine.EngineRunnable.decodeFromSource(EngineRunnable.java:122)
            at com.bumptech.glide.load.engine.EngineRunnable.decode(EngineRunnable.java:101)
            at com.bumptech.glide.load.engine.EngineRunnable.run(EngineRunnable.java:58)
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:422)
            at java.util.concurrent.FutureTask.run(FutureTask.java:237)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:841)
            at com.bumptech.glide.load.engine.executor.FifoPriorityThreadPoolExecutor$DefaultThreadFactory$1.run(FifoPriorityThreadPoolExecutor.java:52)
@TWiStErRob
Copy link
Collaborator Author

On a related note, if you include

compile 'com.github.bumptech.glide:okhttp-integration:1.2.2@aar'

but don't include com.squareup.okhttp:okhttp:2.3.0, the Error comes earlier on the main thread:

    java.lang.NoClassDefFoundError: com.squareup.okhttp.OkHttpClient
            at com.bumptech.glide.integration.okhttp.OkHttpUrlLoader$Factory.getInternalClient(OkHttpUrlLoader.java:30)
            at com.bumptech.glide.integration.okhttp.OkHttpUrlLoader$Factory.<init>(OkHttpUrlLoader.java:41)
            at com.bumptech.glide.integration.okhttp.OkHttpGlideModule.registerComponents(OkHttpGlideModule.java:31)
            at com.bumptech.glide.Glide.get(Glide.java:159)
            at com.bumptech.glide.RequestManager.<init>(RequestManager.java:59)
            at com.bumptech.glide.RequestManager.<init>(RequestManager.java:51)
            at com.bumptech.glide.manager.RequestManagerRetriever.supportFragmentGet(RequestManagerRetriever.java:184)
            at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:103)
            at com.bumptech.glide.Glide.with(Glide.java:633)
            at net.twisterrob.app.GlideActivity.onCreate(GlideActivity.java:25)
            at android.app.Activity.performCreate(Activity.java:5426)
            at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1105)
            at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2269)
            at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2363)
            at android.app.ActivityThread.access$900(ActivityThread.java:161)
            at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1265)
            at android.os.Handler.dispatchMessage(Handler.java:102)
            at android.os.Looper.loop(Looper.java:157)
            at android.app.ActivityThread.main(ActivityThread.java:5356)
            at java.lang.reflect.Method.invokeNative(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:515)
            at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1265)
            at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1081)
            at dalvik.system.NativeStart.main(Native Method)

sjudd added a commit to sjudd/glide that referenced this issue Apr 26, 2015
@sjudd
Add options for handling uncaught exceptions.
4f786ca 
Fixes bumptech#435.
@sjudd sjudd closed this as completed in 2cf8671 Apr 26, 2015
@TWiStErRob TWiStErRob mentioned this issue Jun 10, 2015
Closed
@TWiStErRob TWiStErRob added the bug label Oct 17, 2015
@BigFishStudios
Copy link

I have had the same problem with

compile 'com.github.bumptech.glide:okhttp-integration:1.3.1@aar'

adding
compile('com.squareup.okhttp:okhttp:2.5.0') {
exclude group: 'com.squareup.okio', module: 'okio'

is the current solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@TWiStErRob @BigFishStudios