Merge changes Id0f7deac,I27b6be10,Id24991b4,Ibcac4ab0,I8c8d581f, ...

* changes:
  Remove errFn from save(|Diff|Edit)Preferences
  Remove errFn from getGroupMembers
  Remove errFn from createRepoBranch and createRepoTag
  Remove errFn from deleteRepoTags
  Remove errFn from deleteRepoBranches
  Remove errFn from createGroup
  Remove errFn from createRepo
  Remove errFn from saveRepoConfig and runRepoGC
  Create proper event and util for network-error
  Create proper event and util for server-error

polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts

@@ -128,6 +128,7 @@ export class GrRepoCommands extends GestureEventListeners(
   }
 
   _handleRunningGC() {
+    if (!this.repo) return;
     this._runningGC = true;
     return this.restApiService
       .runRepoGC(this.repo)

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts

@@ -106,7 +106,7 @@ import {GrStorage, StorageLocation} from '../../shared/gr-storage/gr-storage';
 import {isAttentionSetEnabled} from '../../../utils/attention-set-util';
 import {CODE_REVIEW, getMaxAccounts} from '../../../utils/label-util';
 import {isUnresolved} from '../../../utils/comment-util';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireServerError} from '../../../utils/event-util';
 
 const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
 
@@ -721,13 +721,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
           return new Map<AccountId | EmailAddress, boolean>();
         }
         if (!response.ok) {
-          this.dispatchEvent(
+          fireServerError(response);
-            new CustomEvent('server-error', {
-              detail: {response},
-              composed: true,
-              bubbles: true,
-            })
-          );
           return new Map<AccountId | EmailAddress, boolean>();
         }
 
@@ -790,8 +784,9 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     return account._account_id === change.owner._account_id;
   }
 
-  _handle400Error(response?: Response | null) {
+  _handle400Error(r?: Response | null) {
-    if (!response) throw new Error('Reponse is empty.');
+    if (!r) throw new Error('Reponse is empty.');
+    let response: Response = r;
     // A call to _saveReview could fail with a server error if erroneous
     // reviewers were requested. This is signalled with a 400 Bad Request
     // status. The default gr-rest-api-interface error handling would
@@ -813,7 +808,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
       const result = parsed as ReviewResult;
       // Only perform custom error handling for 400s and a parseable
       // ReviewResult response.
-      if (response && response.status === 400 && result && result.reviewers) {
+      if (response.status === 400 && result && result.reviewers) {
         const errors: string[] = [];
         const addReviewers = Object.values(result.reviewers);
         addReviewers.forEach(r => errors.push(r.error ?? 'no explanation'));
@@ -823,13 +818,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
           text: () => Promise.resolve(errors.join(', ')),
         };
       }
-      this.dispatchEvent(
+      fireServerError(response);
-        new CustomEvent('server-error', {
-          detail: {response},
-          composed: true,
-          bubbles: true,
-        })
-      );
     });
   }
 

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js

@@ -21,6 +21,7 @@ import './gr-reply-dialog.js';
 import {mockPromise} from '../../../test/test-utils.js';
 import {SpecialFilePath} from '../../../constants/constants.js';
 import {appContext} from '../../../services/app-context.js';
+import {addListenerForTest} from '../../../test/test-utils.js';
 
 const basicFixture = fixtureFromElement('gr-reply-dialog');
 
@@ -778,23 +779,23 @@ suite('gr-reply-dialog tests', () => {
     assert.isTrue(eraseDraftCommentStub.calledWith(location));
   });
 
-  test('400 converts to human-readable server-error', async () => {
+  test('400 converts to human-readable server-error', done => {
     sinon.stub(window, 'fetch').callsFake(() => {
       const text = '....{"reviewers":{"id1":{"error":"human readable"}}}';
       return Promise.resolve(cloneableResponse(400, text));
     });
 
-    let resolver;
+    const listener = event => {
-    const promise = new Promise(r => resolver = r);
+      if (event.target !== document) return;
-    element.addEventListener('server-error', resolver);
+      event.detail.response.text().then(body => {
+        if (body === 'human readable') {
+          done();
+        }
+      });
+    };
+    addListenerForTest(document, 'server-error', listener);
 
-    await flush();
+    flush(() => { element.send(); });
-    element.send();
-
-    const event = await promise;
-    assert.equal(event.target, element);
-    const text = await event.detail.response.text();
-    assert.equal(text, 'human readable');
   });
 
   test('non-json 400 is treated as a normal server-error', done => {
@@ -803,15 +804,15 @@ suite('gr-reply-dialog tests', () => {
       return Promise.resolve(cloneableResponse(400, text));
     });
 
-    element.addEventListener('server-error', event => {
+    const listener = event => {
-      if (event.target !== element) {
+      if (event.target !== document) return;
-        return;
-      }
       event.detail.response.text().then(body => {
-        assert.equal(body, 'Comment validation error!');
+        if (body === 'Comment validation error!') {
-        done();
+          done();
+        }
       });
-    });
+    };
+    addListenerForTest(document, 'server-error', listener);
 
     // Async tick is needed because iron-selector content is distributed and
     // distributed content requires an observer to be set up.
@@ -1255,12 +1256,13 @@ suite('gr-reply-dialog tests', () => {
         },
       },
     });
-    element.addEventListener('server-error', e => {
+    const listener = e => {
       e.detail.response.text().then(text => {
         assert.equal(text, [error1, error2, error3].join(', '));
         done();
       });
-    });
+    };
+    addListenerForTest(document, 'server-error', listener);
     element._handle400Error(cloneableResponse(400, text));
   });
 

polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts

@@ -34,10 +34,10 @@ import {EventEmitterService} from '../../../services/gr-event-interface/gr-event
 import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
 import {GrErrorDialog} from '../gr-error-dialog/gr-error-dialog';
 import {GrAlert} from '../../shared/gr-alert/gr-alert';
-import {FetchRequest} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
 import {ErrorType, FixIronA11yAnnouncer} from '../../../types/types';
 import {AccountId} from '../../../types/common';
 import {EventType} from '../../../utils/event-util';
+import {NetworkErrorEvent, ServerErrorEvent} from '../../../types/events';
 
 const HIDE_ALERT_TIMEOUT_MS = 5000;
 const CHECK_SIGN_IN_INTERVAL_MS = 60 * 1000;
@@ -125,8 +125,8 @@ export class GrErrorManager extends GestureEventListeners(
   /** @override */
   attached() {
     super.attached();
-    this.listen(document, 'server-error', '_handleServerError');
+    this.listen(document, EventType.SERVER_ERROR, '_handleServerError');
-    this.listen(document, 'network-error', '_handleNetworkError');
+    this.listen(document, EventType.NETWORK_ERROR, '_handleNetworkError');
     this.listen(document, EventType.SHOW_ALERT, '_handleShowAlert');
     this.listen(document, 'hide-alert', '_hideAlert');
     this.listen(document, 'show-error', '_handleShowErrorDialog');
@@ -147,8 +147,8 @@ export class GrErrorManager extends GestureEventListeners(
   detached() {
     super.detached();
     this._clearHideAlertHandle();
-    this.unlisten(document, 'server-error', '_handleServerError');
+    this.unlisten(document, EventType.SERVER_ERROR, '_handleServerError');
-    this.unlisten(document, 'network-error', '_handleNetworkError');
+    this.unlisten(document, EventType.NETWORK_ERROR, '_handleNetworkError');
     this.unlisten(document, EventType.SHOW_ALERT, '_handleShowAlert');
     this.unlisten(document, 'hide-alert', '_hideAlert');
     this.unlisten(document, 'show-error', '_handleShowErrorDialog');
@@ -177,9 +177,7 @@ export class GrErrorManager extends GestureEventListeners(
     });
   }
 
-  _handleServerError(
+  _handleServerError(e: ServerErrorEvent) {
-    e: CustomEvent<{response: Response; request: FetchRequest}>
-  ) {
     const {request, response} = e.detail;
     response.text().then(errorText => {
       const url = request && (request.anonymizedUrl || request.url);
@@ -296,7 +294,7 @@ export class GrErrorManager extends GestureEventListeners(
     );
   }
 
-  _handleNetworkError(e: CustomEvent) {
+  _handleNetworkError(e: NetworkErrorEvent) {
     this._showAlert('Server unavailable');
     console.error(e.detail.error.message);
   }

polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts

@@ -68,7 +68,11 @@ import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
 import {LineNumber, FILE} from '../gr-diff/gr-diff-line';
 import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
 import {KnownExperimentId} from '../../../services/flags/flags';
-import {firePageError, fireAlert} from '../../../utils/event-util';
+import {
+  firePageError,
+  fireAlert,
+  fireServerError,
+} from '../../../utils/event-util';
 
 const MSG_EMPTY_BLAME = 'No blame information for this diff.';
 
@@ -591,13 +595,7 @@ export class GrDiffHost extends GestureEventListeners(
     // Loading the diff may respond with 409 if the file is too large. In this
     // case, use a toast error..
     if (response.status === 409) {
-      this.dispatchEvent(
+      fireServerError(response);
-        new CustomEvent('server-error', {
-          detail: {response},
-          composed: true,
-          bubbles: true,
-        })
-      );
       return;
     }
 

polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js

@@ -304,11 +304,15 @@ suite('gr-diff-host tests', () => {
 
       setup(() => {
         serverErrorStub = sinon.stub();
-        element.addEventListener('server-error', serverErrorStub);
+        document.addEventListener('server-error', serverErrorStub);
         pageErrorStub = sinon.stub();
         element.addEventListener('page-error', pageErrorStub);
       });
 
+      teardown(() => {
+        document.removeEventListener('server-error', serverErrorStub);
+      });
+
       test('page error on HTTP-409', () => {
         element._handleGetDiffError({status: 409});
         assert.isTrue(serverErrorStub.calledOnce);

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

@@ -157,7 +157,7 @@ import {
   HttpMethod,
   ReviewerState,
 } from '../../../constants/constants';
-import {firePageError} from '../../../utils/event-util';
+import {firePageError, fireServerError} from '../../../utils/event-util';
 
 const JSON_PREFIX = ")]}'";
 const MAX_PROJECT_RESULTS = 25;
@@ -408,19 +408,7 @@ export class GrRestApiInterface
     }) as Promise<DashboardInfo[] | undefined>;
   }
 
-  saveRepoConfig(repo: RepoName, config: ConfigInput): Promise<Response>;
+  saveRepoConfig(repo: RepoName, config: ConfigInput): Promise<Response> {
-
-  saveRepoConfig(
-    repo: RepoName,
-    config: ConfigInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  saveRepoConfig(
-    repo: RepoName,
-    config: ConfigInput,
-    errFn?: ErrorCallback
-  ): Promise<Response | undefined> {
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const url = `/projects/${encodeURIComponent(repo)}/config`;
@@ -429,23 +417,11 @@ export class GrRestApiInterface
       method: HttpMethod.PUT,
       url,
       body: config,
-      errFn,
       anonymizedUrl: '/projects/*/config',
     });
   }
 
-  runRepoGC(repo: RepoName): Promise<Response>;
+  runRepoGC(repo: RepoName): Promise<Response> {
-
-  runRepoGC(
-    repo: RepoName,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  runRepoGC(repo: RepoName, errFn?: ErrorCallback) {
-    if (!repo) {
-      // TODO(TS): fix return value
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(repo);
@@ -453,23 +429,11 @@ export class GrRestApiInterface
       method: HttpMethod.POST,
       url: `/projects/${encodeName}/gc`,
       body: '',
-      errFn,
       anonymizedUrl: '/projects/*/gc',
     });
   }
 
-  createRepo(config: ProjectInput & {name: RepoName}): Promise<Response>;
+  createRepo(config: ProjectInput & {name: RepoName}): Promise<Response> {
-
-  createRepo(
-    config: ProjectInput & {name: RepoName},
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  createRepo(config: ProjectInput, errFn?: ErrorCallback) {
-    if (!config.name) {
-      // TODO(TS): Fix return value
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(config.name);
@@ -477,29 +441,16 @@ export class GrRestApiInterface
       method: HttpMethod.PUT,
       url: `/projects/${encodeName}`,
       body: config,
-      errFn,
       anonymizedUrl: '/projects/*',
     });
   }
 
-  createGroup(config: GroupInput & {name: string}): Promise<Response>;
+  createGroup(config: GroupInput & {name: string}): Promise<Response> {
-
-  createGroup(
-    config: GroupInput & {name: string},
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  createGroup(config: GroupInput, errFn?: ErrorCallback) {
-    if (!config.name) {
-      // TODO(TS): Fix return value
-      return '';
-    }
     const encodeName = encodeURIComponent(config.name);
     return this._restApiHelper.send({
       method: HttpMethod.PUT,
       url: `/groups/${encodeName}`,
       body: config,
-      errFn,
       anonymizedUrl: '/groups/*',
     });
   }
@@ -515,19 +466,7 @@ export class GrRestApiInterface
     }) as Promise<GroupInfo | undefined>;
   }
 
-  deleteRepoBranches(repo: RepoName, ref: GitRef): Promise<Response>;
+  deleteRepoBranches(repo: RepoName, ref: GitRef): Promise<Response> {
-
-  deleteRepoBranches(
-    repo: RepoName,
-    ref: GitRef,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  deleteRepoBranches(repo: RepoName, ref: GitRef, errFn?: ErrorCallback) {
-    if (!repo || !ref) {
-      // TODO(TS): fix return value
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(repo);
@@ -536,24 +475,11 @@ export class GrRestApiInterface
       method: HttpMethod.DELETE,
       url: `/projects/${encodeName}/branches/${encodeRef}`,
       body: '',
-      errFn,
       anonymizedUrl: '/projects/*/branches/*',
     });
   }
 
-  deleteRepoTags(repo: RepoName, ref: GitRef): Promise<Response>;
+  deleteRepoTags(repo: RepoName, ref: GitRef): Promise<Response> {
-
-  deleteRepoTags(
-    repo: RepoName,
-    ref: GitRef,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  deleteRepoTags(repo: RepoName, ref: GitRef, errFn?: ErrorCallback) {
-    if (!repo || !ref) {
-      // TODO(TS): fix return type
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(repo);
@@ -562,7 +488,6 @@ export class GrRestApiInterface
       method: HttpMethod.DELETE,
       url: `/projects/${encodeName}/tags/${encodeRef}`,
       body: '',
-      errFn,
       anonymizedUrl: '/projects/*/tags/*',
     });
   }
@@ -571,25 +496,7 @@ export class GrRestApiInterface
     name: RepoName,
     branch: BranchName,
     revision: BranchInput
-  ): Promise<Response>;
+  ): Promise<Response> {
-
-  createRepoBranch(
-    name: RepoName,
-    branch: BranchName,
-    revision: BranchInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  createRepoBranch(
-    name: RepoName,
-    branch: BranchName,
-    revision: BranchInput,
-    errFn?: ErrorCallback
-  ) {
-    if (!name || !branch || !revision) {
-      // TODO(TS) fix return type
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(name);
@@ -598,7 +505,6 @@ export class GrRestApiInterface
       method: HttpMethod.PUT,
       url: `/projects/${encodeName}/branches/${encodeBranch}`,
       body: revision,
-      errFn,
       anonymizedUrl: '/projects/*/branches/*',
     });
   }
@@ -607,25 +513,7 @@ export class GrRestApiInterface
     name: RepoName,
     tag: string,
     revision: TagInput
-  ): Promise<Response>;
+  ): Promise<Response> {
-
-  createRepoTag(
-    name: RepoName,
-    tag: string,
-    revision: TagInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  createRepoTag(
-    name: RepoName,
-    tag: string,
-    revision: TagInput,
-    errFn?: ErrorCallback
-  ) {
-    if (!name || !tag || !revision) {
-      // TODO(TS): Fix return value
-      return '';
-    }
     // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
     // supports it.
     const encodeName = encodeURIComponent(name);
@@ -634,7 +522,6 @@ export class GrRestApiInterface
       method: HttpMethod.PUT,
       url: `/projects/${encodeName}/tags/${encodeTag}`,
       body: revision,
-      errFn,
       anonymizedUrl: '/projects/*/tags/*',
     });
   }
@@ -650,16 +537,12 @@ export class GrRestApiInterface
     );
   }
 
-  getGroupMembers(
+  getGroupMembers(groupName: GroupId | GroupName): Promise<AccountInfo[]> {
-    groupName: GroupId | GroupName,
-    errFn?: ErrorCallback
-  ): Promise<AccountInfo[] | undefined> {
     const encodeName = encodeURIComponent(groupName);
-    return this._restApiHelper.fetchJSON({
+    return (this._restApiHelper.fetchJSON({
       url: `/groups/${encodeName}/members/`,
-      errFn,
       anonymizedUrl: '/groups/*/members',
-    }) as Promise<AccountInfo[] | undefined>;
+    }) as unknown) as Promise<AccountInfo[]>;
   }
 
   getIncludedGroup(
@@ -865,14 +748,7 @@ export class GrRestApiInterface
     });
   }
 
-  savePreferences(prefs: PreferencesInput): Promise<Response>;
+  savePreferences(prefs: PreferencesInput): Promise<Response> {
-
-  savePreferences(
-    prefs: PreferencesInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  savePreferences(prefs: PreferencesInput, errFn?: ErrorCallback) {
     // Note (Issue 5142): normalize the download scheme with lower case before
     // saving.
     if (prefs.download_scheme) {
@@ -883,45 +759,28 @@ export class GrRestApiInterface
       method: HttpMethod.PUT,
       url: '/accounts/self/preferences',
       body: prefs,
-      errFn,
       reportUrlAsIs: true,
     });
   }
 
-  saveDiffPreferences(prefs: DiffPreferenceInput): Promise<Response>;
+  saveDiffPreferences(prefs: DiffPreferenceInput): Promise<Response> {
-
-  saveDiffPreferences(
-    prefs: DiffPreferenceInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  saveDiffPreferences(prefs: DiffPreferenceInput, errFn?: ErrorCallback) {
     // Invalidate the cache.
     this._cache.delete('/accounts/self/preferences.diff');
     return this._restApiHelper.send({
       method: HttpMethod.PUT,
       url: '/accounts/self/preferences.diff',
       body: prefs,
-      errFn,
       reportUrlAsIs: true,
     });
   }
 
-  saveEditPreferences(prefs: EditPreferencesInfo): Promise<Response>;
+  saveEditPreferences(prefs: EditPreferencesInfo): Promise<Response> {
-
-  saveEditPreferences(
-    prefs: EditPreferencesInfo,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
-  saveEditPreferences(prefs: EditPreferencesInfo, errFn?: ErrorCallback) {
     // Invalidate the cache.
     this._cache.delete('/accounts/self/preferences.edit');
     return this._restApiHelper.send({
       method: HttpMethod.PUT,
       url: '/accounts/self/preferences.edit',
       body: prefs,
-      errFn,
       reportUrlAsIs: true,
     });
   }
@@ -1494,13 +1353,7 @@ export class GrRestApiInterface
             if (errFn) {
               errFn.call(null, response);
             } else {
-              document.dispatchEvent(
+              fireServerError(response, req);
-                new CustomEvent('server-error', {
-                  detail: {request: req, response},
-                  composed: true,
-                  bubbles: true,
-                })
-              );
             }
             return undefined;
           }
@@ -2207,14 +2060,8 @@ export class GrRestApiInterface
     // 404s indicate the file does not exist yet in the revision, so suppress
     // them.
     const suppress404s: ErrorCallback = res => {
-      if (res?.status !== 404) {
+      if (res && res?.status !== 404) {
-        document.dispatchEvent(
+        fireServerError(res);
-          new CustomEvent('server-error', {
-            detail: {res},
-            composed: true,
-            bubbles: true,
-          })
-        );
       }
       return res;
     };

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

@@ -1294,7 +1294,7 @@ suite('gr-rest-api-interface tests', () => {
         })
         .then(() => {
           assert.isTrue(spy.called);
-          assert.notEqual(spy.lastCall.args[0].detail.res.status, 404);
+          assert.notEqual(spy.lastCall.args[0].detail.response.status, 404);
         });
   });
 

polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts

@@ -32,6 +32,8 @@ import {
 } from '../../../../types/common';
 import {HttpMethod} from '../../../../constants/constants';
 import {RpcLogEventDetail} from '../../../../types/events';
+import {fireNetworkError, fireServerError} from '../../../../utils/event-util';
+import {FetchRequest} from '../../../../types/types';
 
 const JSON_PREFIX = ")]}'";
 
@@ -188,12 +190,6 @@ export interface SendJSONRequest extends SendRequestBase {
 
 export type SendRequest = SendRawRequest | SendJSONRequest;
 
-export interface FetchRequest {
-  url: string;
-  fetchOptions?: AuthRequestInit;
-  anonymizedUrl?: string;
-}
-
 export interface FetchJSONRequest extends FetchRequest {
   reportUrlAsIs?: boolean;
   params?: FetchParams;
@@ -315,13 +311,7 @@ s   */
         if (req.errFn) {
           req.errFn.call(undefined, null, err);
         } else {
-          document.dispatchEvent(
+          fireNetworkError(err);
-            new CustomEvent('network-error', {
-              detail: {error: err},
-              composed: true,
-              bubbles: true,
-            })
-          );
         }
         throw err;
       });
@@ -350,13 +340,7 @@ s   */
           req.errFn.call(null, response);
           return;
         }
-        document.dispatchEvent(
+        fireServerError(response, req);
-          new CustomEvent('server-error', {
-            detail: {request: req, response},
-            composed: true,
-            bubbles: true,
-          })
-        );
         return;
       }
       return this.getResponseObject(response);
@@ -510,13 +494,7 @@ s   */
     };
     const xhr = this.fetch(fetchReq)
       .catch(err => {
-        document.dispatchEvent(
+        fireNetworkError(err);
-          new CustomEvent('network-error', {
-            detail: {error: err},
-            composed: true,
-            bubbles: true,
-          })
-        );
         if (req.errFn) {
           return req.errFn.call(undefined, null, err);
         } else {
@@ -529,13 +507,7 @@ s   */
             req.errFn.call(undefined, response);
             return;
           }
-          document.dispatchEvent(
+          fireServerError(response, fetchReq);
-            new CustomEvent('server-error', {
-              detail: {request: fetchReq, response},
-              composed: true,
-              bubbles: true,
-            })
-          );
         }
         return response;
       });

polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts

@@ -232,26 +232,10 @@ export interface RestApiService {
   getDiffPreferences(): Promise<DiffPreferencesInfo | undefined>;
 
   saveDiffPreferences(prefs: DiffPreferenceInput): Promise<Response>;
-  saveDiffPreferences(
-    prefs: DiffPreferenceInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-  saveDiffPreferences(
-    prefs: DiffPreferenceInput,
-    errFn?: ErrorCallback
-  ): Promise<Response>;
 
   getEditPreferences(): Promise<EditPreferencesInfo | undefined>;
 
   saveEditPreferences(prefs: EditPreferencesInfo): Promise<Response>;
-  saveEditPreferences(
-    prefs: EditPreferencesInfo,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-  saveEditPreferences(
-    prefs: EditPreferencesInfo,
-    errFn?: ErrorCallback
-  ): Promise<Response>;
 
   getAccountEmails(): Promise<EmailInfo[] | undefined>;
   deleteAccountEmail(email: string): Promise<Response>;
@@ -267,25 +251,11 @@ export interface RestApiService {
     revision: BranchInput
   ): Promise<Response>;
 
-  createRepoBranch(
-    name: RepoName,
-    branch: BranchName,
-    revision: BranchInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-
   createRepoTag(
     name: RepoName,
     tag: string,
     revision: TagInput
   ): Promise<Response>;
-
-  createRepoTag(
-    name: RepoName,
-    tag: string,
-    revision: TagInput,
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
   addAccountGPGKey(key: GpgKeysInput): Promise<Record<string, GpgKeyInfo>>;
   deleteAccountGPGKey(id: GpgKeyId): Promise<Response>;
   getAccountGPGKeys(): Promise<Record<string, GpgKeyInfo>>;
@@ -325,11 +295,6 @@ export interface RestApiService {
   ): Promise<ProjectAccessInfo | undefined>;
 
   createRepo(config: ProjectInput & {name: RepoName}): Promise<Response>;
-  createRepo(
-    config: ProjectInput & {name: RepoName},
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-  createRepo(config: ProjectInput, errFn?: ErrorCallback): Promise<Response>;
 
   getRepo(
     repo: RepoName,
@@ -522,11 +487,6 @@ export interface RestApiService {
     | Promise<PathToCommentsInfoMap | undefined>;
 
   createGroup(config: GroupInput & {name: string}): Promise<Response>;
-  createGroup(
-    config: GroupInput & {name: string},
-    errFn: ErrorCallback
-  ): Promise<Response | undefined>;
-  createGroup(config: GroupInput, errFn?: ErrorCallback): Promise<Response>;
 
   getPlugins(
     filter: string,
@@ -660,10 +620,7 @@ export interface RestApiService {
     errFn?: ErrorCallback
   ): Promise<GroupAuditEventInfo[] | undefined>;
 
-  getGroupMembers(
+  getGroupMembers(groupName: GroupId | GroupName): Promise<AccountInfo[]>;
-    groupName: GroupId | GroupName,
-    errFn?: ErrorCallback
-  ): Promise<AccountInfo[] | undefined>;
 
   getIncludedGroup(
     groupName: GroupId | GroupName
@@ -690,10 +647,7 @@ export interface RestApiService {
     includedGroup: GroupId
   ): Promise<Response>;
 
-  runRepoGC(
+  runRepoGC(repo: RepoName): Promise<Response>;
-    repo: RepoName,
-    errFn?: ErrorCallback
-  ): Promise<Response | undefined>;
   getFileContent(
     changeNum: NumericChangeId,
     path: string,

polygerrit-ui/app/test/test-utils.ts

@@ -105,6 +105,25 @@ export function registerTestCleanup(cleanupCallback: CleanupCallback) {
   cleanups.push(cleanupCallback);
 }
 
+export function addListenerForTest(
+  el: EventTarget,
+  type: string,
+  listener: EventListenerOrEventListenerObject
+) {
+  el.addEventListener(type, listener);
+  registerListenerCleanup(el, type, listener);
+}
+
+export function registerListenerCleanup(
+  el: EventTarget,
+  type: string,
+  listener: EventListenerOrEventListenerObject
+) {
+  registerTestCleanup(() => {
+    el.removeEventListener(type, listener);
+  });
+}
+
 export function cleanupTestUtils() {
   cleanups.forEach(cleanup => cleanup());
   cleanups.splice(0);

polygerrit-ui/app/types/events.ts

@@ -19,6 +19,7 @@ import {PatchSetNum} from './common';
 import {UIComment} from '../utils/comment-util';
 import {Side} from '../constants/constants';
 import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
+import {FetchRequest} from './types';
 
 export interface TitleChangeEventDetail {
   title: string;
@@ -44,6 +45,31 @@ declare global {
   }
 }
 
+export interface ServerErrorEventDetail {
+  request?: FetchRequest;
+  response: Response;
+}
+
+export type ServerErrorEvent = CustomEvent<ServerErrorEventDetail>;
+
+declare global {
+  interface DocumentEventMap {
+    'server-error': ServerErrorEvent;
+  }
+}
+
+export interface NetworkErrorEventDetail {
+  error: Error;
+}
+
+export type NetworkErrorEvent = CustomEvent<NetworkErrorEventDetail>;
+
+declare global {
+  interface DocumentEventMap {
+    'network-error': NetworkErrorEvent;
+  }
+}
+
 export interface LocationChangeEventDetail {
   hash: string;
   pathname: string;

polygerrit-ui/app/types/types.ts

@@ -27,6 +27,7 @@ import {
   PatchSetNum,
 } from './common';
 import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
+import {AuthRequestInit} from '../services/gr-auth/gr-auth';
 
 export function notUndefined<T>(x: T | undefined): x is T {
   return x !== undefined;
@@ -237,3 +238,9 @@ export function isPolymerSpliceChange<
 >(x: T | PolymerSpliceChange<U>): x is PolymerSpliceChange<U> {
   return (x as PolymerSpliceChange<U>).indexSplices !== undefined;
 }
+
+export interface FetchRequest {
+  url: string;
+  fetchOptions?: AuthRequestInit;
+  anonymizedUrl?: string;
+}

polygerrit-ui/app/utils/event-util.ts

@@ -15,9 +15,13 @@
  * limitations under the License.
  */
 
+import {FetchRequest} from '../types/types';
+
 export enum EventType {
   SHOW_ALERT = 'show-alert',
   PAGE_ERROR = 'page-error',
+  SERVER_ERROR = 'server-error',
+  NETWORK_ERROR = 'network-error',
   TITLE_CHANGE = 'title-change',
 }
 
@@ -41,6 +45,26 @@ export function firePageError(target: EventTarget, response?: Response | null) {
   );
 }
 
+export function fireServerError(response: Response, request?: FetchRequest) {
+  document.dispatchEvent(
+    new CustomEvent(EventType.SERVER_ERROR, {
+      detail: {response, request},
+      composed: true,
+      bubbles: true,
+    })
+  );
+}
+
+export function fireNetworkError(error: Error) {
+  document.dispatchEvent(
+    new CustomEvent(EventType.NETWORK_ERROR, {
+      detail: {error},
+      composed: true,
+      bubbles: true,
+    })
+  );
+}
+
 export function fireTitleChange(target: EventTarget, title: string) {
   target.dispatchEvent(
     new CustomEvent(EventType.TITLE_CHANGE, {