Standardise the error handling for parsing of JSON response messages from the server. Fixes #3238

docs/en_US/release_notes_3_1.rst

@@ -18,6 +18,7 @@ Bug fixes
 | `Bug #3179 <https://redmine.postgresql.org/issues/3179>`_ - Fix an error generating SQL for trigger functions
 | `Bug #3257 <https://redmine.postgresql.org/issues/3257>`_ - Catch errors when trying to EXPLAIN an invalid query
 | `Bug #3290 <https://redmine.postgresql.org/issues/3290>`_ - Close button added to the alertify message box, which pops up in case of backend error
-| `Bug #3306 <https://redmine.postgresql.org/issues/3306>`_ - Fixed display SQL of table with index for GreenPlum database
+| `Bug #3306 <https://redmine.postgresql.org/issues/3306>`_ - Fixed display SQL of table with index for Greenplum database
-| `Bug #3308 <https://redmine.postgresql.org/issues/3308>`_ - Fixed issue where icon for Partitioned tables was the same as Non Partitioned tables for GreenPlum database
+| `Bug #3308 <https://redmine.postgresql.org/issues/3308>`_ - Fixed issue where icon for Partitioned tables was the same as Non Partitioned tables for Greenplum database
-| `Bug #3324 <https://redmine.postgresql.org/issues/3324>`_ - Fix the template loader to work reliably under Windows (fixing external tables under Greenplum)
+| `Bug #3238 <https://redmine.postgresql.org/issues/3238>`_ - Standardise the error handling for parsing of JSON response messages from the server.
+| `Bug #3324 <https://redmine.postgresql.org/issues/3324>`_ - Fix the template loader to work reliably under Windows (fixing external tables under Greenplum)

web/pgadmin/browser/server_groups/servers/databases/casts/static/js/cast.js

@@ -209,15 +209,8 @@ define('pgadmin.node.cast', [
                 },
 
                  // On failure show error appropriate error message to user
-                error: function(xhr) {
+                error: function(xhr, status, error) {
-                  try {
+                  alertify.pgRespErrorNotify(xhr, error);
-                    var err = $.parseJSON(xhr.responseText);
-                    if (err.success == 0) {
-                      alertify.error(err.errormsg);
-                    }
-                  } catch (e) {
-                    // Do nothing
-                  }
                 },
               });
             }

web/pgadmin/browser/server_groups/servers/databases/schemas/synonyms/static/js/synonym.js

@@ -163,15 +163,8 @@ define('pgadmin.node.synonym', [
               },
 
                   // On failure show error appropriate error message to user
-              error: function(xhr) {
+              error: function(xhr, status, error) {
-                try {
+                alertify.pgRespErrorNotify(xhr, error);
-                  var err = $.parseJSON(xhr.responseText);
-                  if (err.success == 0) {
-                    alertify.error(err.errormsg);
-                  }
-                } catch (e) {
-                  // Ignore
-                }
               },
             });
             return res;

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/check_constraint/static/js/check_constraint.js

@@ -73,15 +73,8 @@ define('pgadmin.node.check_constraint', [
                 setTimeout(function() {t.select(i);}, 100);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.js

@@ -665,15 +665,8 @@ define('pgadmin.node.foreign_key', [
                 setTimeout(function() {t.select(i);}, 100);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/partitions/static/js/partition.js

@@ -174,15 +174,8 @@ function(
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });
@@ -233,15 +226,8 @@ function(
                       }, 10);
                     }
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                     t.unload(i);
                   },
                 });
@@ -283,15 +269,8 @@ function(
                       }, 10);
                     }
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                     t.unload(i);
                   },
                 });
@@ -335,15 +314,8 @@ function(
                       }
                     }
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                   },
                 });
               }
@@ -941,9 +913,8 @@ function(
                       );
                     }
                   },
-                  error: function(e) {
+                  error: function(xhr, status, error) {
-                    var errmsg = $.parseJSON(e.responseText);
+                    Alertify.pgRespErrorNotify(xhr, error, gettext('Error fetching tables to be attached'));
-                    Alertify.alert(gettext('Error fetching tables to be attached'), errmsg.errormsg);
                   },
                 });
               }

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.js

@@ -153,15 +153,8 @@ define('pgadmin.node.table', [
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });
@@ -212,15 +205,8 @@ define('pgadmin.node.table', [
                       }, 10);
                     }
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                     t.unload(i);
                   },
                 });
@@ -262,15 +248,8 @@ define('pgadmin.node.table', [
                       }, 10);
                     }
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                     t.unload(i);
                   },
                 });
@@ -302,15 +281,8 @@ define('pgadmin.node.table', [
                 t.select(i);
               }, 10);
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });
@@ -1047,9 +1019,8 @@ define('pgadmin.node.table', [
                       );
                     }
                   },
-                  error: function(e) {
+                  error: function(xhr, status, error) {
-                    var errmsg = $.parseJSON(e.responseText);
+                    Alertify.pgRespErrorNotify(xhr, error);
-                    Alertify.alert(gettext('Error fetching tables to be attached'), errmsg.errormsg);
                   },
                 });
               }

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/static/js/trigger.js

@@ -130,15 +130,8 @@ define('pgadmin.node.trigger', [
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });
@@ -175,15 +168,8 @@ define('pgadmin.node.trigger', [
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              alertify.pgRespErrorNotify(xhr, error, gettext('Disable trigger failed'));
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });

web/pgadmin/browser/server_groups/servers/databases/schemas/views/static/js/mview.js

@@ -300,9 +300,8 @@ define('pgadmin.node.mview', [
               );
             }
           },
-          error: function(e) {
+          error: function(xhr, status, error) {
-            var errmsg = $.parseJSON(e.responseText);
+            Alertify.pgRespErrorNotify(xhr, error, gettext('Error refreshing view'));
-            Alertify.alert(gettext('Error refreshing view'), errmsg.errormsg);
           },
         });
 

web/pgadmin/browser/server_groups/servers/databases/static/js/database.js

@@ -208,15 +208,8 @@ define('pgadmin.node.database', [
                     t.unload(i);
                   }
                 },
-                error: function(xhr) {
+                error: function(xhr, status, error) {
-                  try {
+                  Alertify.pgRespErrorNotify(xhr, error);
-                    var err = $.parseJSON(xhr.responseText);
-                    if (err.success == 0) {
-                      Alertify.error(err.errormsg);
-                    }
-                  } catch (e) {
-                    console.warn(e.stack || e);
-                  }
                   t.unload(i);
                 },
               });

web/pgadmin/browser/server_groups/servers/pgagent/static/js/pga_job.js

@@ -175,24 +175,8 @@ define('pgadmin.node.pga_job', [
           // 'pgagent.pga_job' table updated with current time to run the job
           // now.
           success: function() { t.unload(i); },
-          error: function(xhr) {
+          error: function(xhr, status, error) {
-            var error_msg = gettext('Unable to run pgagent job.');
+            alertify.pgRespErrorNotify(xhr, error);
-            if (xhr.readyState == 0) {
-              alertify.error(
-                gettext('Not connected to the server or the connection to the server has been closed.')
-              );
-            }
-            else {
-              if (_.isUndefined(xhr.responseText)) {
-                alertify.error(error_msg);
-              }
-              else {
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  alertify.error(err.errormsg);
-                }
-              }
-            }
             t.unload(i);
           },
         });

web/pgadmin/browser/server_groups/servers/static/js/server.js

@@ -217,15 +217,8 @@ define('pgadmin.node.server', [
                   t.unload(i);
                 }
               },
-              error: function(xhr) {
+              error: function(xhr, status, error) {
-                try {
+                Alertify.pgRespErrorNotify(xhr, error);
-                  var err = $.parseJSON(xhr.responseText);
-                  if (err.success == 0) {
-                    Alertify.error(err.errormsg);
-                  }
-                } catch (e) {
-                  console.warn(e.stack || e);
-                }
                 t.unload(i);
               },
             });
@@ -299,15 +292,8 @@ define('pgadmin.node.server', [
                     Alertify.error(res.data.result);
                   }
                 },
-                error: function(xhr) {
+                error: function(xhr, status, error) {
-                  try {
+                  Alertify.pgRespErrorNotify(xhr, error);
-                    var err = $.parseJSON(xhr.responseText);
-                    if (err.success == 0) {
-                      Alertify.error(err.errormsg);
-                    }
-                  } catch (e) {
-                    console.warn(e.stack || e);
-                  }
                   t.unload(i);
                 },
               });
@@ -342,15 +328,8 @@ define('pgadmin.node.server', [
                   success: function(res) {
                     Alertify.success(res.data.result, 10);
                   },
-                  error: function(xhr) {
+                  error: function(xhr, status, error) {
-                    try {
+                    Alertify.pgRespErrorNotify(xhr, error);
-                      var err = $.parseJSON(xhr.responseText);
-                      if (err.success == 0) {
-                        Alertify.error(err.errormsg, 10);
-                      }
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
                     t.unload(i);
                   },
                 });
@@ -520,15 +499,8 @@ define('pgadmin.node.server', [
                           Alertify.error(res.errormsg);
                         }
                       },
-                      error: function(xhr) {
+                      error: function(xhr, status, error) {
-                        try {
+                        Alertify.pgRespErrorNotify(xhr, error);
-                          var err = $.parseJSON(xhr.responseText);
-                          if (err.success == 0) {
-                            Alertify.error(err.errormsg);
-                          }
-                        } catch (e) {
-                          console.warn(e.stack || e);
-                        }
                       },
                     });
                   }
@@ -547,15 +519,8 @@ define('pgadmin.node.server', [
               }
               Alertify.changeServerPassword(d).resizeTo('40%','52%');
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
             },
           });
 
@@ -590,15 +555,8 @@ define('pgadmin.node.server', [
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });
@@ -632,15 +590,8 @@ define('pgadmin.node.server', [
                 }, 10);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
               t.unload(i);
             },
           });

web/pgadmin/browser/server_groups/servers/tablespaces/static/js/tablespace.js

@@ -285,15 +285,8 @@ define('pgadmin.node.tablespace', [
                               Alertify.error(res.errormsg);
                             }
                           },
-                          error: function(xhr) {
+                          error: function(xhr, status, error) {
-                            try {
+                            Alertify.pgRespErrorNotify(xhr, error);
-                              var err = $.parseJSON(xhr.responseText);
-                              if (err.success == 0) {
-                                Alertify.error(err.errormsg);
-                              }
-                            } catch (e) {
-                              console.warn(e.stack || e);
-                            }
                           },
                         });
                       },

web/pgadmin/browser/static/js/browser.js

@@ -691,13 +691,8 @@ define('pgadmin.browser', [
           modifyAnimation.modifyAcitreeAnimation(self);
           modifyAnimation.modifyAlertifyAnimation(self);
         },
-        error: function(xhr) {
+        error: function(xhr, status, error) {
-          try {
+          Alertify.pgRespErrorNotify(xhr, error);
-            var err = $.parseJSON(xhr.responseText);
-            Alertify.alert(gettext('Preference loading failed.'),
-              err.errormsg
-            );
-          } catch (e) { console.warn(e.stack || e); }
         },
       });
     },
@@ -1826,13 +1821,8 @@ define('pgadmin.browser', [
               }
               fetchNodeInfo(_callback);
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) { console.warn(e.stack || e); }
               fetchNodeInfo(_callback);
             },
           });

web/pgadmin/dashboard/static/js/dashboard.js

@@ -97,15 +97,8 @@ define('pgadmin.dashboard', [
                 Alertify.error(txtError);
               }
             },
-            error: function(xhr) {
+            error: function(xhr, status, error) {
-              try {
+              Alertify.pgRespErrorNotify(xhr, error);
-                var err = $.parseJSON(xhr.responseText);
-                if (err.success == 0) {
-                  Alertify.error(err.errormsg);
-                }
-              } catch (e) {
-                console.warn(e.stack || e);
-              }
             },
           });
         },

web/pgadmin/static/js/alertify.pgadmin.defaults.js

@@ -139,6 +139,45 @@ define([
     ).set('title', promptmsg).set('closable', true);
   };
 
+  alertify.pgRespErrorNotify = (xhr, error, prefixMsg='') => {
+    var contentType = xhr.getResponseHeader('Content-Type');
+    try {
+      if (xhr.status === 0) {
+        error = gettext('Connection to the server has been lost.');
+      } else {
+        if(contentType){
+          if(contentType.indexOf('application/json') >= 0) {
+            var resp = JSON.parse(xhr.responseText);
+            error = _.escape(resp.result) || _.escape(resp.errormsg) || gettext('Unknown error');
+          }
+        }
+        if (contentType.indexOf('text/html') >= 0) {
+          var alertMessage = '\
+                <div class="media font-red-3 text-14">\
+                  <div class="media-body media-middle">\
+                    <div class="alert-text">' + gettext('INTERNAL SERVER ERROR') + '</div><br/>\
+                    <div class="alert-text">' + gettext('Click for details.') + '</div>\
+                  </div>\
+                </div>';
+
+          alertify.notify(
+            alertMessage, 'error', 0, () => {
+              alertify.pgIframeDialog()
+                .show()
+                .set({frameless: false})
+                .set('pg_msg', xhr.responseText);
+            }
+          );
+          return;
+        }
+      }
+    }
+    catch(e){
+      error = e.message;
+    }
+    alertify.error(prefixMsg +' '+error);
+  };
+
   var alertifyDialogResized = function(stop) {
     var self = this;
 

web/regression/javascript/alerts/alertify_wrapper_spec.js

@@ -7,6 +7,9 @@
 //
 //////////////////////////////////////////////////////////////////////////
 import alertify from 'pgadmin.alertifyjs';
+import * as $ from 'jquery';
+import gettext from 'sources/gettext';
+
 
 describe('alertify_wrapper', function () {
   describe('success', function () {
@@ -34,4 +37,81 @@ describe('alertify_wrapper', function () {
       expect(calledWithMessage).toContain('class="fa fa-exclamation-triangle"');
     });
   });
+
+  describe('pgRespErrorNotify', () => {
+    it('calls error notifier which alertifies response error for ajax calls', () => {
+
+      $.ajax({
+        url: 'http://some/dummy/url',
+        dataType: 'json',
+        error: function(xhr, status, error) {
+
+          spyOn(alertify, 'orig_error').and.callThrough();
+          spyOn(alertify, 'notify').and.callThrough();
+
+          /*When connection lost*/
+          xhr.status = 0;
+          alertify.pgRespErrorNotify(xhr, error);
+          expect(alertify.orig_error).toHaveBeenCalled();
+          expect(alertify.orig_error.calls.mostRecent().args[0]).toContain(
+            gettext('Connection to the server has been lost.')
+          );
+
+
+          /*When some exception occurs at back end*/
+          xhr.status = 4;
+          var orig_getResponseHeader = xhr.getResponseHeader;
+
+          /*Exception handled by back end*/
+          xhr.getResponseHeader = (header) => {
+            if(header === 'Content-Type') {
+              return 'application/json';
+            }
+            else {
+              return orig_getResponseHeader(header);
+            }
+
+          };
+          xhr.responseText = '{"errormsg":"Exception XYZ"}';
+          alertify.pgRespErrorNotify(xhr, error);
+          expect(alertify.orig_error).toHaveBeenCalled();
+          expect(alertify.orig_error.calls.mostRecent().args[0]).toContain(
+            gettext('Exception XYZ')
+          );
+
+          /*Exception not handled by back end*/
+          xhr.getResponseHeader = (header) => {
+            if(header === 'Content-Type') {
+              return 'text/html';
+            }
+            else {
+              return orig_getResponseHeader(header);
+            }
+          };
+          xhr.responseText = '<p>Some Exception Occurred</p>';
+          alertify.pgRespErrorNotify(xhr, error);
+          expect(alertify.notify).toHaveBeenCalled();
+          expect(alertify.notify.calls.mostRecent().args[0]).toContain(
+            gettext('INTERNAL SERVER ERROR')
+          );
+
+          /*With prefixMsg*/
+          xhr.getResponseHeader = (header) => {
+            if(header === 'Content-Type') {
+              return 'application/json';
+            }
+            else {
+              return orig_getResponseHeader(header);
+            }
+          };
+          xhr.responseText = '{"errormsg":"Exception XYZ"}';
+          alertify.pgRespErrorNotify(xhr, error, gettext('Some prefix message'));
+          expect(alertify.orig_error).toHaveBeenCalled();
+          expect(alertify.orig_error.calls.mostRecent().args[0]).toContain(
+            gettext('Some prefix message')
+          );
+        },
+      });
+    });
+  });
 });