GNOME Bugzilla – Bug 763038
souphttpsrc: add http error code to element error messages
Last modified: 2016-07-25 10:18:58 UTC
+++ This bug was initially created as a clone of Bug #756806 +++ Add http error code to souphttpsrc element error messages.
Created attachment 322965 [details] [review] include http error code in error message
Created attachment 322974 [details] [review] include http error code in error message HTTP error code is now unsigned.
Created attachment 331734 [details] [review] include http error code in error message Uses the latest patch from https://bugzilla.gnome.org/show_bug.cgi?id=756806
Review of attachment 331734 [details] [review]: ::: ext/soup/gstsouphttpsrc.c @@ +1228,3 @@ ("Server does not accept Range HTTP header, URL: %s, Redirect to: %s", + src->location, GST_STR_NULL (src->redirection_uri)), + ("http-error-code", G_TYPE_UINT, msg->status_code, NULL)); It's an HTTP status code, not error code :) It might also be a good idea to add the message body here, it can contain useful information.
Created attachment 331999 [details] [review] include http status code in error message
Created attachment 332000 [details] [review] add body when available
Comment on attachment 331999 [details] [review] include http status code in error message Might make sense to also add the redirection URL to the details though
Comment on attachment 332000 [details] [review] add body when available On a second thought this might be not a good idea at all... the body might be huge, and especially when printed in debug logs, error messages, etc it might kill applications :)
Created attachment 332081 [details] [review] include http status code in error message With http-redirect-uri too.
Review of attachment 332081 [details] [review]: ::: ext/soup/gstsouphttpsrc.c @@ +1228,3 @@ ("Server does not accept Range HTTP header, URL: %s, Redirect to: %s", + src->location, GST_STR_NULL (src->redirection_uri)), + ("http-status-code", G_TYPE_UINT, msg->status_code, NULL)); This and the others not using the SOUP_HTTP_SRC_ERROR() macro are not adding the redirect-uri
Created attachment 332087 [details] [review] include http status code in error message
Review of attachment 332087 [details] [review]: ::: ext/soup/gstsouphttpsrc.c @@ +1228,3 @@ ("Server does not accept Range HTTP header, URL: %s, Redirect to: %s", + src->location, GST_STR_NULL (src->redirection_uri)), + ("http-status-code", G_TYPE_UINT, msg->status_code, NULL)); redirection uri? please also double-check if you really got all places where the status code should be added
Well, this error is a seek error, not a redirect one. Not sure why it's also including a redirection uri...
Created attachment 332093 [details] [review] include http status code in error message Added. http-status-code is present in all relevant calls to GST_ELEMENT_ERROR*. The ones it's not in are things like no URL given, or library errors.
A seek is just another HTTP request :) You could e.g. seek before anything else and then the first request is a Range request that might cause a redirect or two to happen and then something fails.
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Thu Mar 3 11:35:06 2016 +0000 souphttpsrc: include http-status-code in error message details https://bugzilla.gnome.org/show_bug.cgi?id=763038