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.

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

GlideException doesn't return anymore HTTP response code #1967

Closed
fasteque opened this issue May 24, 2017 · 12 comments
Closed

GlideException doesn't return anymore HTTP response code #1967

fasteque opened this issue May 24, 2017 · 12 comments
Labels
Milestone

Comments

@fasteque
Copy link

Glide Version: 4.0.0-RC0
Integration libraries: okhttp3-integration:4.0.0-RC0
Device/Android Version: Android TV emulator X86, Android 7.0
Issue details / Repro steps / Use case background:
I'm upgrading my application from version 3.8.0 to the 4.0.0-RC0 and I have to manage errors when I load images from a remote service, which require authentication.
In 3.8.0, once the session is expired or not valid anymore, the service reply with a 401 error and what I get in the Glide listener is a IOException and the message is the original sent from the server, so I can do a very simple check on it and look for 401.

Example: Request failed with code: 401

In 4.0.0-RC0, the GlideException cannot be casted anymore to IOException (instanceof returns false) but even worse the message of the exception is a generic Failed to load resource or something similar.

So I cannot anymore recognize different type of HTTP errors and react accordingly.

Am I missing something new introduced in the 4.0.0-RC0? I really need this kind of information, so the moment I'll stick to the 3.8.0 or eventually switch to another library.

Glide load line / GlideModule (if any) / list Adapter code (if any):

RequestOptions options = new RequestOptions()
		.format(PREFER_ARGB_8888);

GlideApp.with(this)
		.asBitmap()
		.apply(options)
		.load(getGlideUrl(thumbnailUrl))
		.listener(new RequestListener<Bitmap>() {
			@Override
			public boolean onLoadFailed(@Nullable GlideException e, Object model, Target<Bitmap>
					target, boolean isFirstResource) {
				removeLoadingOverlay();

				// When the session is expired or not valid anymore.
				if (e != null) {
                                        // check exception
				} else {
                                        // manage error without exception					
				}
				return false;
			}

			@Override
			public boolean onResourceReady(Bitmap resource, Object model, Target<Bitmap> target,
										   DataSource dataSource, boolean isFirstResource) {
				removeLoadingOverlay();
				return false;
			}
		})
		.into(imageView);
@sjudd
Copy link
Collaborator

sjudd commented May 24, 2017

http://bumptech.github.io/glide/javadocs/400/com/bumptech/glide/load/engine/GlideException.html#getRootCauses--

If you'd like to add a section to the migrating documentation I'd welcome a pull request, see http://bumptech.github.io/glide/dev/contributing.html#documentation

@sjudd sjudd closed this as completed May 24, 2017
@sjudd sjudd added the question label May 24, 2017
@fasteque
Copy link
Author

Ok thanks, I've seen that method in the documentation, but it's returning an empty list.

@TWiStErRob TWiStErRob reopened this May 24, 2017
@TWiStErRob
Copy link
Collaborator

@fasteque
Copy link
Author

Sure, I come back to you when I have something, thanks.

@fasteque
Copy link
Author

fasteque commented May 24, 2017

Note: I have removed the code to set OkHttp in my custom GlideModule.

I don't get to that point or in general that class.

"main@4347" prio=5 tid=0x1 nid=NA runnable
  java.lang.Thread.State: RUNNABLE
	  at com.swisscom.mycloud.tv.slideshow.SlideshowFragment$3.onLoadFailed(SlideshowFragment.java:200)
	  at com.bumptech.glide.request.SingleRequest.onLoadFailed(SingleRequest.java:516)
	  at com.bumptech.glide.request.SingleRequest.onLoadFailed(SingleRequest.java:500)
	  at com.bumptech.glide.load.engine.EngineJob.handleExceptionOnMainThread(EngineJob.java:266)
	  at com.bumptech.glide.load.engine.EngineJob$MainThreadCallback.handleMessage(EngineJob.java:298)
	  at android.os.Handler.dispatchMessage(Handler.java:98)
	  at android.os.Looper.loop(Looper.java:154)
	  at android.app.ActivityThread.main(ActivityThread.java:6077)
	  at java.lang.reflect.Method.invoke(Method.java:-1)
	  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
	  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)

Logs:

05-24 15:38:15.127 5251-5251/com.swisscom.mycloud.dev E/Glide: class com.bumptech.glide.load.engine.GlideException: Failed to load resource
05-24 15:38:18.334 1305-1333/? W/audio_hw_generic: Not supplying enough data to HAL, expected position 13927722 , only wrote 13670802

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented May 24, 2017

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented May 24, 2017

@sjudd swallowing a non-2xx error code like that is nasty. It is not a dropin-compatible replacement for the default fetcher, I think HttpException should be "thown" there.

  /**
   * Returns true if the code is in [200..300), which means the request was successfully received,
   * understood, and accepted.
   */
  public boolean isSuccessful() {
    return code >= 200 && code < 300;
  }

@fasteque
Copy link
Author

fasteque commented May 24, 2017

Breakpoint in onResponse for OkHttpStreamFetcher gets triggered for my requests, so OkHttp3 integration is working fine, but as soon as I kill my session server side and I get the GlideException in the listener, onResponse is not even called (of course I expect this before the exception). Stack trace is the same as I've copied above.

@fasteque
Copy link
Author

This is where the failure notification is sent for my use case:

callback = {EngineJob@6970} 
 cbs = {ArrayList@7024}  size = 1
 dataSource = null
 decodeJob = {DecodeJob@6965} 
 diskCacheExecutor = {GlideExecutor@7025} "com.bumptech.glide.load.engine.executor.GlideExecutor@fa5d196[Running, pool size = 1, active threads = 0, queued tasks = 0, completed tasks = 31]"
 engineResource = null
 engineResourceFactory = {EngineJob$EngineResourceFactory@7026} 
 exception = null
 hasLoadFailed = false
 hasResource = false
 ignoredCallbacks = null
 isCacheable = true
 isCancelled = false
 key = {EngineKey@6980} "EngineKey{model=https://library.dev.mdl.swisscom.ch/thumbnail/Storage::Photos::Asset::OGdFYWZYYm9VMDA5RzJOZmtzczRnQT09?height=2160&access_token=c786yVown0+x3nDITQ2V+Q==, width=1920, height=1080, resourceClass=class android.graphics.Bitmap, transcodeClass=class android.graphics.Bitmap, signature=EmptySignature, hashCode=-705657996, transformations={class android.graphics.Bitmap=com.bumptech.glide.load.resource.bitmap.FitCenter@5db7ce1d, class com.bumptech.glide.load.resource.gif.GifDrawable=com.bumptech.glide.load.resource.gif.GifDrawableTransformation@5db7ce1d, class android.graphics.drawable.BitmapDrawable=com.bumptech.glide.load.resource.bitmap.BitmapDrawableTransformation@5db7ce1d}, options=Options{values={Option{key='com.bumptech.glide.load.resource.bitmap.Downsampler.DownsampleStrategy'}=com.bumptech.glide.load.resource.bitmap.DownsampleStrategy$FitCenter@e3a3e8a, Option{key='com.bumptech.glide.load.resource.bitmap.Downsampler.DecodeFormat'}=PREFER_ARGB_8888}}}"
 listener = {Engine@7027} 
 pool = {FactoryPools$FactoryPool@7028} 
 resource = null
 sourceExecutor = {GlideExecutor@6966} "com.bumptech.glide.load.engine.executor.GlideExecutor@76e9f29[Running, pool size = 4, active threads = 1, queued tasks = 0, completed tasks = 17]"
 sourceUnlimitedExecutor = {GlideExecutor@7029} "com.bumptech.glide.load.engine.executor.GlideExecutor@e57122[Running, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]"
 stateVerifier = {StateVerifier$DefaultStateVerifier@7030} 
 useUnlimitedSourceGeneratorPool = false
 shadow$_klass_ = {Class@6350} "class com.bumptech.glide.load.engine.EngineJob"
 shadow$_monitor_ = -1943942180

@fasteque
Copy link
Author

Actually you can easily test on your side too, just passing an invalid access token

i.e. : https://library.dev.mdl.swisscom.ch/thumbnail/Storage::Photos::Asset::dDBSRDZFbVlQUDFlYkZIdmZTRkJsUT09?height=2160&access_token=dfsdafsfdf

If you open this in Chrome you see Status Code: 401; Unauthorized as expected.

@sjudd
Copy link
Collaborator

sjudd commented Jun 2, 2017

I'll try and take a look. I'd expect the logging in the OkHttp response only gets called if onError is also called? I'm not sure what it means if OkHttp's onError callback is not called but the response is not successful. I'd assume any failure would trigger the onError callback.

@sjudd sjudd added this to the 4.0 milestone Jun 2, 2017
@sjudd
Copy link
Collaborator

sjudd commented Jun 2, 2017

Ok I think I get it, onError is called if the request itself fails (connection failure), onResponse is called otherwise if the request is made. Easy to fix, thanks for the report!

@sjudd sjudd closed this as completed in b52a23a Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants