[libgdata] [core] Improved default error handling for insertions, updates and deletions



commit 185026570b0887ea94f769f9a53462b237df0ed8
Author: Philip Withnall <philip tecnocode co uk>
Date:   Sat May 9 16:51:52 2009 +0100

    [core] Improved default error handling for insertions, updates and deletions
    
    Wrote a better implementation of real_parse_error_response, which will examine
    the HTTP status code to choose a more appropriate error message. Failing that,
    it will fall back to generic error messages for each different action
    (insertion, update, etc.).
    
    This has changed the API of GDataServiceClass->parse_error_response.
---
 gdata/gdata-service.c                          |   82 +++++++++++++++++++-----
 gdata/gdata-service.h                          |   29 +++++----
 gdata/services/youtube/gdata-youtube-service.c |    9 ++-
 3 files changed, 88 insertions(+), 32 deletions(-)

diff --git a/gdata/gdata-service.c b/gdata/gdata-service.c
index 077290a..83e4205 100644
--- a/gdata/gdata-service.c
+++ b/gdata/gdata-service.c
@@ -64,7 +64,7 @@ static void gdata_service_get_property (GObject *object, guint property_id, GVal
 static void gdata_service_set_property (GObject *object, guint property_id, const GValue *value, GParamSpec *pspec);
 static gboolean real_parse_authentication_response (GDataService *self, guint status, const gchar *response_body, gint length, GError **error);
 static void real_append_query_headers (GDataService *self, SoupMessage *message);
-static void real_parse_error_response (GDataService *self, guint status, const gchar *reason_phrase,
+static void real_parse_error_response (GDataService *self, GDataServiceError error_type, guint status, const gchar *reason_phrase,
 				       const gchar *response_body, gint length, GError **error);
 static void notify_proxy_uri_cb (GObject *gobject, GParamSpec *pspec, GObject *self);
 
@@ -327,10 +327,57 @@ real_append_query_headers (GDataService *self, SoupMessage *message)
 }
 
 static void
-real_parse_error_response (GDataService *self, guint status, const gchar *reason_phrase, const gchar *response_body, gint length, GError **error)
+real_parse_error_response (GDataService *self, GDataServiceError error_type, guint status, const gchar *reason_phrase, const gchar *response_body,
+			   gint length, GError **error)
 {
-	g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_QUERY,
-		     _("Error code %u when querying: %s"), status, reason_phrase);
+	/* See: http://code.google.com/apis/gdata/docs/2.0/reference.html#HTTPStatusCodes */
+
+	switch (status) {
+		case 400:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_PROTOCOL_ERROR,
+				     _("Invalid request URI or header, or unsupported nonstandard parameter: %s"), reason_phrase);
+			return;
+		case 401:
+		case 403:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_AUTHENTICATION_REQUIRED,
+				     _("Authentication required: %s"), reason_phrase);
+			return;
+		case 404:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_NOT_FOUND,
+				     _("The requested resource was not found: %s"), reason_phrase);
+			return;
+		case 409:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_CONFLICT,
+				     _("The entry has been modified since it was downloaded: %s"), reason_phrase);
+			return;
+		case 500:
+		default:
+			/* We'll fall back to generic errors, below */
+			break;
+	}
+
+	/* If the error hasn't been handled already, throw a generic error corresponding to the action type */
+	switch (error_type) {
+		case GDATA_SERVICE_ERROR_WITH_INSERTION:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_INSERTION,
+				     _("Error code %u when inserting an entry: %s"), status, reason_phrase);
+			break;
+		case GDATA_SERVICE_ERROR_WITH_UPDATE:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_UPDATE,
+				     _("Error code %u when updating an entry: %s"), status, reason_phrase);
+			break;
+		case GDATA_SERVICE_ERROR_WITH_DELETION:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_DELETION,
+				     _("Error code %u when deleting an entry: %s"), status, reason_phrase);
+			break;
+		case GDATA_SERVICE_ERROR_WITH_QUERY:
+			g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_QUERY,
+				     _("Error code %u when querying: %s"), status, reason_phrase);
+			break;
+		default:
+			/* We should not be called with anything other than the above four generic error types */
+			g_assert_not_reached ();
+	}
 }
 
 typedef struct {
@@ -524,6 +571,7 @@ authenticate (GDataService *self, const gchar *username, const gchar *password,
 		const gchar *response_body = message->response_body->data;
 		gchar *error_start, *error_end, *uri_start, *uri_end, *uri = NULL;
 
+		/* Parse the error response; see: http://code.google.com/apis/accounts/docs/AuthForInstalledApps.html#Errors */
 		if (response_body == NULL)
 			goto protocol_error;
 
@@ -586,9 +634,9 @@ authenticate (GDataService *self, const gchar *username, const gchar *password,
 			return authenticate (self, username, password, g_strndup (captcha_start, captcha_end - captcha_start), new_captcha_answer,
 					     cancellable, error);
 		} else if (strncmp (error_start, "Unknown", error_end - error_start) == 0) {
-			goto protocol_error; /* TODO: is this really a protocol error? It's an error with *our* code */
+			goto protocol_error;
 		} else if (strncmp (error_start, "BadAuthentication", error_end - error_start) == 0) {
-			/* TODO: Looks like Error=BadAuthentication errors don't return a URI */
+			/* Looks like Error=BadAuthentication errors don't return a URI */
 			g_set_error_literal (error, GDATA_AUTHENTICATION_ERROR, GDATA_AUTHENTICATION_ERROR_BAD_AUTHENTICATION,
 					     _("Your username or password were incorrect."));
 			goto login_error;
@@ -953,7 +1001,8 @@ gdata_service_query (GDataService *self, const gchar *feed_uri, GDataQuery *quer
 	} else if (status != 200) {
 		/* Error */
 		g_assert (klass->parse_error_response != NULL);
-		klass->parse_error_response (self, status, message->reason_phrase, message->response_body->data, message->response_body->length, error);
+		klass->parse_error_response (self, GDATA_SERVICE_ERROR_WITH_QUERY, status, message->reason_phrase, message->response_body->data,
+					     message->response_body->length, error);
 		g_object_unref (message);
 		return NULL;
 	}
@@ -1051,9 +1100,9 @@ gdata_service_insert_entry (GDataService *self, const gchar *upload_uri, GDataEn
 
 	if (status != 201) {
 		/* Error */
-		/* TODO: Handle errors more specifically, making sure to set authenticated where appropriate */
-		g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_INSERTION,
-			     _("error code %u when inserting"), status);
+		g_assert (klass->parse_error_response != NULL);
+		klass->parse_error_response (self, GDATA_SERVICE_ERROR_WITH_INSERTION, status, message->reason_phrase, message->response_body->data,
+					     message->response_body->length, error);
 		g_object_unref (message);
 		return NULL;
 	}
@@ -1137,9 +1186,9 @@ gdata_service_update_entry (GDataService *self, GDataEntry *entry, GCancellable
 
 	if (status != 200) {
 		/* Error */
-		/* TODO: Handle errors more specifically, making sure to set authenticated where appropriate */
-		g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_UPDATE,
-			     _("error code %u when updating"), status);
+		g_assert (klass->parse_error_response != NULL);
+		klass->parse_error_response (self, GDATA_SERVICE_ERROR_WITH_UPDATE, status, message->reason_phrase, message->response_body->data,
+					     message->response_body->length, error);
 		g_object_unref (message);
 		return NULL;
 	}
@@ -1212,9 +1261,10 @@ gdata_service_delete_entry (GDataService *self, GDataEntry *entry, GCancellable
 
 	if (status != 200) {
 		/* Error */
-		/* TODO: Handle errors more specifically, making sure to set authenticated where appropriate */
-		g_set_error (error, GDATA_SERVICE_ERROR, GDATA_SERVICE_ERROR_WITH_DELETION,
-			     _("error code %u when deleting"), status);
+		g_assert (klass->parse_error_response != NULL);
+		klass->parse_error_response (self, GDATA_SERVICE_ERROR_WITH_DELETION, status, message->reason_phrase, message->response_body->data,
+					     message->response_body->length, error);
+		g_object_unref (message);
 		return FALSE;
 	}
 
diff --git a/gdata/gdata-service.h b/gdata/gdata-service.h
index c7ab779..f43b54f 100644
--- a/gdata/gdata-service.h
+++ b/gdata/gdata-service.h
@@ -33,35 +33,40 @@ G_BEGIN_DECLS
  * GDataServiceError:
  * @GDATA_SERVICE_ERROR_UNAVAILABLE: The service is unavailable due to maintainence or other reasons
  * @GDATA_SERVICE_ERROR_PROTOCOL_ERROR: The client or server unexpectedly strayed from the protocol (fatal error)
- * @GDATA_SERVICE_ERROR_WITH_QUERY: TODO
+ * @GDATA_SERVICE_ERROR_WITH_QUERY: Generic error when querying for entries
  * @GDATA_SERVICE_ERROR_ENTRY_ALREADY_INSERTED: An entry has already been inserted, and cannot be re-inserted
- * @GDATA_SERVICE_ERROR_WITH_INSERTION: TODO
+ * @GDATA_SERVICE_ERROR_WITH_INSERTION: Generic error when inserting an entry
  * @GDATA_SERVICE_ERROR_AUTHENTICATION_REQUIRED: The user attempted to do something which required authentication, and they weren't authenticated
- * @GDATA_SERVICE_ERROR_WITH_UPDATE: TODO
- * @GDATA_SERVICE_ERROR_WITH_DELETION: TODO
+ * @GDATA_SERVICE_ERROR_WITH_UPDATE: Generic error when updating an entry
+ * @GDATA_SERVICE_ERROR_WITH_DELETION: Generic error when deleting an entry
+ * @GDATA_SERVICE_ERROR_NOT_FOUND: A requested resource (feed or entry) was not found on the server
+ * @GDATA_SERVICE_ERROR_CONFLICT: There was a conflict when updating an entry on the server; the server-side copy was modified inbetween downloading
+ * and uploading the modified entry
  *
  * Error codes for #GDataService operations.
  **/
 typedef enum {
 	GDATA_SERVICE_ERROR_UNAVAILABLE = 1,
 	GDATA_SERVICE_ERROR_PROTOCOL_ERROR,
-	GDATA_SERVICE_ERROR_WITH_QUERY,/* TODO: probably should die */
+	GDATA_SERVICE_ERROR_WITH_QUERY,
 	GDATA_SERVICE_ERROR_ENTRY_ALREADY_INSERTED,
-	GDATA_SERVICE_ERROR_WITH_INSERTION,/* TODO: probably should die */
+	GDATA_SERVICE_ERROR_WITH_INSERTION,
 	GDATA_SERVICE_ERROR_AUTHENTICATION_REQUIRED,
-	GDATA_SERVICE_ERROR_WITH_UPDATE,/* TODO: probably should die */
-	GDATA_SERVICE_ERROR_WITH_DELETION/* TODO: probably should die */
+	GDATA_SERVICE_ERROR_WITH_UPDATE,
+	GDATA_SERVICE_ERROR_WITH_DELETION,
+	GDATA_SERVICE_ERROR_NOT_FOUND,
+	GDATA_SERVICE_ERROR_CONFLICT
 } GDataServiceError;
 
 /**
  * GDataAuthenticationError:
  * @GDATA_AUTHENTICATION_ERROR_BAD_AUTHENTICATION: The login request used a username or password that is not recognized.
  * @GDATA_AUTHENTICATION_ERROR_NOT_VERIFIED: The account email address has not been verified. The user will need to access their Google account
-    directly to resolve the issue before logging in using a non-Google application.
+ * directly to resolve the issue before logging in using a non-Google application.
  * @GDATA_AUTHENTICATION_ERROR_TERMS_NOT_AGREED: The user has not agreed to terms. The user will need to access their Google account directly to
-    resolve the issue before logging in using a non-Google application.
+ * resolve the issue before logging in using a non-Google application.
  * @GDATA_AUTHENTICATION_ERROR_CAPTCHA_REQUIRED: A CAPTCHA is required. (A response with this error code will also contain an image URL and a
-    CAPTCHA token.)
+ * CAPTCHA token.)
  * @GDATA_AUTHENTICATION_ERROR_ACCOUNT_DELETED: The user account has been deleted.
  * @GDATA_AUTHENTICATION_ERROR_ACCOUNT_DISABLED: The user account has been disabled.
  * @GDATA_AUTHENTICATION_ERROR_SERVICE_DISABLED: The user's access to the specified service has been disabled. (The user account may still be valid.)
@@ -139,7 +144,7 @@ typedef struct {
 
 	gboolean (*parse_authentication_response) (GDataService *self, guint status, const gchar *response_body, gint length, GError **error);
 	void (*append_query_headers) (GDataService *self, SoupMessage *message);
-	void (*parse_error_response) (GDataService *self, guint status, const gchar *reason_phrase,
+	void (*parse_error_response) (GDataService *self, GDataServiceError error_type, guint status, const gchar *reason_phrase,
 				      const gchar *response_body, gint length, GError **error);
 } GDataServiceClass;
 
diff --git a/gdata/services/youtube/gdata-youtube-service.c b/gdata/services/youtube/gdata-youtube-service.c
index 4fb865f..1a0e344 100644
--- a/gdata/services/youtube/gdata-youtube-service.c
+++ b/gdata/services/youtube/gdata-youtube-service.c
@@ -54,7 +54,7 @@ static void gdata_youtube_service_get_property (GObject *object, guint property_
 static void gdata_youtube_service_set_property (GObject *object, guint property_id, const GValue *value, GParamSpec *pspec);
 static gboolean parse_authentication_response (GDataService *self, guint status, const gchar *response_body, gint length, GError **error);
 static void append_query_headers (GDataService *self, SoupMessage *message);
-static void parse_error_response (GDataService *self, guint status, const gchar *reason_phrase,
+static void parse_error_response (GDataService *self, GDataServiceError error_type, guint status, const gchar *reason_phrase,
 				  const gchar *response_body, gint length, GError **error);
 
 struct _GDataYouTubeServicePrivate {
@@ -222,7 +222,8 @@ append_query_headers (GDataService *self, SoupMessage *message)
 }
 
 static void
-parse_error_response (GDataService *self, guint status, const gchar *reason_phrase, const gchar *response_body, gint length, GError **error)
+parse_error_response (GDataService *self, GDataServiceError error_type, guint status, const gchar *reason_phrase, const gchar *response_body,
+		      gint length, GError **error)
 {
 	xmlDoc *doc;
 	xmlNode *node;
@@ -346,7 +347,7 @@ parse_error_response (GDataService *self, guint status, const gchar *reason_phra
 
 parent:
 	/* Chain up to the parent class */
-	GDATA_SERVICE_CLASS (gdata_youtube_service_parent_class)->parse_error_response (self, status, reason_phrase,
+	GDATA_SERVICE_CLASS (gdata_youtube_service_parent_class)->parse_error_response (self, error_type, status, reason_phrase,
 											response_body, length, error);
 	return;
 }
@@ -728,7 +729,7 @@ gdata_youtube_service_upload_video (GDataYouTubeService *self, GDataYouTubeVideo
 
 	if (status != 201) {
 		/* Error */
-		parse_error_response (GDATA_SERVICE (self), status, message->reason_phrase,
+		parse_error_response (GDATA_SERVICE (self), GDATA_SERVICE_ERROR_WITH_INSERTION, status, message->reason_phrase,
 				      message->response_body->data, message->response_body->length, error);
 		g_object_unref (message);
 		return NULL;



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]