Merge "Propagate error's text received from backend to a plugin"

polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts

@@ -19,6 +19,20 @@ import {HttpMethod} from '../../../constants/constants';
 import {RequestPayload} from '../../../types/common';
 import {appContext} from '../../../services/app-context';
 
+async function getErrorMessage(response: Response): Promise<string> {
+  const text = await response.text();
+  return text || `${response.status}`;
+}
+
+// This is an internal error, that must never be visible outside of this
+// file. It is used only inside GrPluginRestApi.send method. See detailed
+// explanation in the GrPluginRestApi.send method.
+class ResponseError extends Error {
+  public constructor(readonly response: Response) {
+    super();
+  }
+}
+
 export class GrPluginRestApi {
   private readonly restApi = appContext.restApiService;
 
@@ -114,11 +128,18 @@ export class GrPluginRestApi {
       errFn ??
       ((response: Response | null | undefined, error?: Error) => {
         if (error) throw error;
-        if (response) throw new Error(`${response.status}`);
+        // Some plugins show an error message if send is failed, smth like:
+        // pluginApi.send(...).catch(err => showError(err));
+        // The response can contain an error text, but getting this text is
+        // an asynchronous operation. At the same time, the errFn must be a
+        // synchronous function.
+        // As a workaround, we throw an ResponseError here and then catch
+        // it inside a catch block below and read the message.
+        if (response) throw new ResponseError(response);
         throw new Error('Generic REST API error.');
       });
-    return this.fetch(method, url, payload, errFn, contentType).then(
-      response => {
+    return this.fetch(method, url, payload, errFn, contentType)
+      .then(response => {
         // Will typically not happen. The response can only be unset, if the
         // errFn handles the error and then returns void or undefined or null.
         // But the errFn above always throws.
@@ -128,18 +149,21 @@ export class GrPluginRestApi {
         // Will typically not happen. errFn will have dealt with that and the
         // caller will get a rejected promise already.
         if (response.status < 200 || response.status >= 300) {
-          return response.text().then(text => {
-            if (text) {
-              return Promise.reject(new Error(text));
-            } else {
-              return Promise.reject(new Error(`${response.status}`));
-            }
-          });
+          return getErrorMessage(response).then(msg =>
+            Promise.reject(new Error(msg))
+          );
         } else {
           return this.restApi.getResponseObject(response);
         }
-      }
-    );
+      })
+      .catch(err => {
+        if (err instanceof ResponseError) {
+          return getErrorMessage(err.response).then(msg => {
+            throw new Error(msg);
+          });
+        }
+        throw err;
+      });
   }
 
   get(url: string) {

polygerrit-ui/app/polymer.json

@@ -9,6 +9,10 @@
   ],
   "lint": {
     "rules": ["polymer-3"],
-    "ignoreWarnings": ["deprecated-dom-call"]
+    "ignoreWarnings": ["deprecated-dom-call"],
+    "filesToIgnore": [
+        "**/gr-plugin-rest-api.js",
+        "**/.cache/**/gr-plugin-rest-api.js"
+    ]
   }
 }