Handle restAPI errors
The pluginStatus property is added to the CodeOwnersModel class. The
ModelLoader catches all exceptions and setup failed pluginStatus when
a request fails.
If a fail happens, the error is displayed:
* At the top of page
* Near the "Code owners" label
The banner is required due to the following reason: If the code-owners
plugin configuration has a problem, the "Code owners" label can be
hidden (backend doesn't return the label). But we still want to display
a error message, so the code uses banner to show the error.
Change-Id: I0701b269ae32d2727e2749324a664f071eec9c80
diff --git a/ui/code-owners-banner.js b/ui/code-owners-banner.js
new file mode 100644
index 0000000..a44e258
--- /dev/null
+++ b/ui/code-owners-banner.js
@@ -0,0 +1,176 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {PluginState} from './code-owners-model.js';
+
+// There are 2 elements in this file:
+// CodeOwnersBanner - visual elements. This element is shown at the top
+// of the page when pluginStatus are changed to failed.
+// The CodeOwnersBanner is added to the 'banner' endpoint. The endpoint
+// is placed at the top level of the gerrit and doesn't provide a current
+// change, so it is impossible to get the pluginStatus directly in the
+// CodeOwnersBanner.
+// To solve this problem, this files provides non-visual
+// CodeOwnersPluginStatusNotifier element. This element is added to the
+// change-view-integration endpoint, so it can track the plugin state.
+// When a plugin state is changed, the CodeOwnersPluginStatusNotifier updates
+// the pluginStatus property of the banner.
+
+// CodeOwnersBanner and CodeOwnersPluginStatusNotifier can be attached and
+// detached in any order. The following piece of code ensures that these
+// banner and notifier are always connected correctly. The code assumes,
+// that max one banner and one notifier can be connected at any time.
+let activeBanner = undefined;
+let activeStatusNotifier = undefined;
+
+function setActiveBanner(banner) {
+ // banner is null when CodeOwnersBanner has been disconnected
+ activeBanner = banner;
+ if (activeStatusNotifier) {
+ activeStatusNotifier.banner = banner;
+ }
+}
+
+function setActiveStatusNotifier(notifier) {
+ // notifier is null when CodeOwnersBanner has been disconnected
+ if (activeStatusNotifier) {
+ if (activeStatusNotifier.banner) {
+ activeStatusNotifier.banner.pluginStatus = undefined;
+ }
+ activeStatusNotifier.banner = undefined;
+ }
+ activeStatusNotifier = notifier;
+ if (activeStatusNotifier) {
+ activeStatusNotifier.banner = activeBanner;
+ }
+}
+
+export class CodeOwnersBanner extends Polymer.Element {
+ static get is() { return 'gr-code-owners-banner'; }
+
+ static get template() {
+ return Polymer.html`
+ <style include="shared-styles">
+ :host {
+ display: block;
+ overflow: hidden;
+ background: red;
+ }
+ .text {
+ color: white;
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h3);
+ font-weight: var(--font-weight-h3);
+ line-height: var(--line-height-h3);
+ margin-left: var(--spacing-l);
+ }
+ </style>
+ <span class="text">Error: Code-owners plugin has failed</span>
+ <gr-button link on-click="_showFailDetails">
+ Details
+ </gr-button>
+ `;
+ }
+
+ static get properties() {
+ return {
+ // @internal attributes
+ hidden: {
+ type: Boolean,
+ value: true,
+ reflectToAttribute: true,
+ computed: '_computeHidden(pluginStatus)',
+ },
+ pluginStatus: {
+ type: Object,
+ },
+ };
+ }
+
+ connectedCallback() {
+ super.connectedCallback();
+ setActiveBanner(this);
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ setActiveBanner(undefined);
+ }
+
+ _computeHidden(pluginStatus) {
+ return !pluginStatus || pluginStatus.state !== PluginState.Failed;
+ }
+
+ _showFailDetails() {
+ showPluginFailedMessage(this, this.pluginStatus);
+ }
+}
+
+customElements.define(CodeOwnersBanner.is, CodeOwnersBanner);
+
+export class CodeOwnersPluginStatusNotifier extends
+ CodeOwnersModelMixin(Polymer.Element) {
+ static get is() {
+ return 'owners-plugin-status-notifier';
+ }
+
+ static get properties() {
+ return {
+ banner: {
+ type: Object,
+ },
+ };
+ }
+
+ static get observers() {
+ return [
+ '_pluginStatusOrBannerChanged(model.pluginStatus, banner)',
+ ];
+ }
+
+ connectedCallback() {
+ super.connectedCallback();
+ setActiveStatusNotifier(this);
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ setActiveStatusNotifier(undefined);
+ }
+
+ _pluginStatusOrBannerChanged(status, banner) {
+ if (!banner) return;
+ banner.pluginStatus = status;
+ }
+
+ _loadDataAfterStateChanged() {
+ this.modelLoader.loadPluginStatus();
+ }
+}
+
+customElements.define(CodeOwnersPluginStatusNotifier.is,
+ CodeOwnersPluginStatusNotifier);
+
+export function showPluginFailedMessage(sourceEl, pluginStatus) {
+ sourceEl.dispatchEvent(
+ new CustomEvent('show-error', {
+ detail: {message: pluginStatus.failedMessage},
+ composed: true,
+ bubbles: true,
+ })
+ );
+}
diff --git a/ui/code-owners-model-loader.js b/ui/code-owners-model-loader.js
index 2074fcb..2cd9095 100644
--- a/ui/code-owners-model-loader.js
+++ b/ui/code-owners-model-loader.js
@@ -35,7 +35,13 @@
async _loadProperty(propertyName, propertyLoader, propertySetter) {
// Load property only if it was not already loaded
if (this.ownersModel[propertyName] !== undefined) return;
- const newValue = await propertyLoader();
+ let newValue;
+ try {
+ newValue = await propertyLoader();
+ } catch (e) {
+ this.ownersModel.setPluginFailed(e.message);
+ return;
+ }
// It is possible, that several requests is made in parallel.
// Store only the first result and discard all other results.
// (also, due to the CodeOwnersCacheApi all result must be identical)
@@ -64,10 +70,10 @@
);
}
- async loadIsCodeOwnerEnabled() {
- await this._loadProperty('isCodeOwnerEnabled',
+ async loadPluginStatus() {
+ await this._loadProperty('pluginStatus',
() => this.ownersService.isCodeOwnerEnabled(),
- value => this.ownersModel.setIsCodeOwnerEnabled(value)
+ value => this.ownersModel.setPluginEnabled(value)
);
}
@@ -84,14 +90,26 @@
!== SuggestionsState.NotLoaded) return;
this.ownersModel.setSuggestionsState(SuggestionsState.Loading);
- const suggestedOwners = await this.ownersService.getSuggestedOwners();
+ let suggestedOwners;
+ try {
+ suggestedOwners = await this.ownersService.getSuggestedOwners();
+ } catch (e) {
+ this.ownersModel.setSuggestionsState(SuggestionsState.LoadFailed);
+ this.ownersModel.setPluginFailed(e.message);
+ return;
+ }
this.ownersModel.setSuggestions(suggestedOwners.suggestions);
this.ownersModel.setSuggestionsState(SuggestionsState.Loaded);
}
async updateLoadSuggestionsProgress() {
- const suggestedOwners =
- await this.ownersService.getSuggestedOwnersProgress();
+ let suggestedOwners;
+ try {
+ suggestedOwners = await this.ownersService.getSuggestedOwnersProgress();
+ } catch {
+ // Ignore any error, keep progress unchanged.
+ return;
+ }
this.ownersModel.setSuggestionsLoadProgress(suggestedOwners.progress);
this.ownersModel.setSuggestions(suggestedOwners.suggestions);
}
diff --git a/ui/code-owners-model.js b/ui/code-owners-model.js
index 4c63419..cbd9bd2 100644
--- a/ui/code-owners-model.js
+++ b/ui/code-owners-model.js
@@ -19,6 +19,13 @@
NotLoaded: 'NotLoaded',
Loaded: 'Loaded',
Loading: 'Loading',
+ LoadFailed: 'LoadFailed',
+};
+
+export const PluginState = {
+ Enabled: 'Enabled',
+ Disabled: 'Disabled',
+ Failed: 'Failed',
};
/**
@@ -51,6 +58,7 @@
this.suggestionsState = SuggestionsState.NotLoaded;
this.suggestionsLoadProgress = undefined;
this.showSuggestions = false;
+ this.pluginStatus = undefined;
}
setBranchConfig(config) {
@@ -107,6 +115,31 @@
this._firePropertyChanged('showSuggestions');
}
+ setPluginEnabled(enabled) {
+ this._setPluginStatus({state: enabled ?
+ PluginState.Enabled : PluginState.Disabled});
+ }
+
+ setPluginFailed(failedMessage) {
+ this._setPluginStatus({state: PluginState.Failed, failedMessage});
+ }
+
+ _setPluginStatus(status) {
+ if (this._arePluginStatusesEqual(this.pluginStatus, status)) return;
+ this.pluginStatus = status;
+ this._firePropertyChanged('pluginStatus');
+ }
+
+ _arePluginStatusesEqual(status1, status2) {
+ if (status1 === undefined || status2 === undefined) {
+ return status1 === status2;
+ }
+ if (status1.state !== status2.state) return false;
+ return status1.state === PluginState.Failed ?
+ status1.failedMessage === status2.failedMessage :
+ true;
+ }
+
_firePropertyChanged(propertyName) {
this.dispatchEvent(new CustomEvent('model-property-changed', {
detail: {
diff --git a/ui/code-owners-service.js b/ui/code-owners-service.js
index 9b670b9..86fbfcc 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -302,8 +302,16 @@
* Prefetch data
*/
async prefetch() {
- this.codeOwnerCacheApi.getAccount();
- this.getStatus();
+ try {
+ await Promise.all([
+ this.codeOwnerCacheApi.getAccount(),
+ this.getStatus(),
+ ]);
+ } catch {
+ // Ignore any errors during prefetch.
+ // The same call from a different place throws the same exception
+ // again. The CodeOwnerService is not responsible for error processing.
+ }
}
/**
@@ -567,7 +575,7 @@
return false;
}
const config = await this.codeOwnerCacheApi.getBranchConfig();
- return !(config.status && config.status.disabled);
+ return config && !(config.status && config.status.disabled);
}
static getOwnerService(restApi, change) {
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index c9d9cb8..230577c 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -17,6 +17,8 @@
import {OwnerStatus} from './code-owners-service.js';
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {showPluginFailedMessage} from './code-owners-banner.js';
+import {PluginState} from './code-owners-model.js';
/**
* Owner requirement control for `submit-requirement-item-code-owners` endpoint.
@@ -62,18 +64,26 @@
Loading status ...
</p>
<template is="dom-if" if="[[!_isLoading]]">
- <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
- <template is="dom-if" if="[[_overrideInfoUrl]]">
- <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]"
- target="_blank">
- <iron-icon icon="gr-icons:help-outline"
- title="Documentation for overriding code owners"></iron-icon>
- </a>
+ <template is="dom-if" if="[[!_pluginFailed(model.pluginStatus)]]">
+ <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
+ <template is="dom-if" if="[[_overrideInfoUrl]]">
+ <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]"
+ target="_blank">
+ <iron-icon icon="gr-icons:help-outline"
+ title="Documentation for overriding code owners"></iron-icon>
+ </a>
+ </template>
+ <template is="dom-if" if="[[!_allApproved]]">
+ <gr-button link on-click="_openReplyDialog">
+ Suggest owners
+ </gr-button>
+ </template>
</template>
- <template is="dom-if" if="[[!_allApproved]]">
- <gr-button link on-click="_openReplyDialog">
- Suggest owners
- </gr-button>
+ <template is="dom-if" if="[[_pluginFailed(model.pluginStatus)]]">
+ <span>Code-owners plugin has failed</span>
+ <gr-button link on-click="_showFailDetails">
+ Details
+ </gr-button>
</template>
</template>
`;
@@ -85,7 +95,7 @@
_isLoading: {
type: Boolean,
computed: '_computeIsLoading(model.branchConfig, model.status, '
- + 'model.userRole)',
+ + 'model.userRole, model.pluginStatus)',
},
_allApproved: {
type: Boolean,
@@ -116,10 +126,17 @@
this.modelLoader.loadUserRole();
}
- _computeIsLoading(branchConfig, status, userRole) {
+ _computeIsLoading(branchConfig, status, userRole, pluginStatus) {
+ if (this._pluginFailed(pluginStatus)) {
+ return false;
+ }
return !branchConfig || !status || !userRole;
}
+ _pluginFailed(pluginStatus) {
+ return pluginStatus && pluginStatus.state === PluginState.Failed;
+ }
+
_onStatusChanged(status, userRole) {
if (!status || !userRole) {
this._statusCount = undefined;
@@ -160,7 +177,7 @@
}
_computeAllApproved(statusCount) {
- return statusCount.missing === 0
+ return statusCount && statusCount.missing === 0
&& statusCount.pending === 0;
}
@@ -225,6 +242,10 @@
this.reporting.reportInteraction('suggest-owners-from-submit-requirement',
{user_role: this.model.userRole});
}
+
+ _showFailDetails() {
+ showPluginFailedMessage(this, this.model.pluginStatus);
+ }
}
customElements.define(OwnerRequirementValue.is, OwnerRequirementValue);
diff --git a/ui/plugin.js b/ui/plugin.js
index 1a278dd..40bc4e5 100644
--- a/ui/plugin.js
+++ b/ui/plugin.js
@@ -19,11 +19,21 @@
import {OwnerStatusColumnContent, OwnerStatusColumnHeader} from './owner-status-column.js';
import {OwnerRequirementValue} from './owner-requirement.js';
import {SuggestOwnersTrigger} from './suggest-owners-trigger.js';
+import {CodeOwnersBanner, CodeOwnersPluginStatusNotifier} from './code-owners-banner.js';
Gerrit.install(plugin => {
const restApi = plugin.restApi();
const reporting = plugin.reporting();
+ plugin.registerCustomComponent('banner', CodeOwnersBanner.is);
+
+ plugin.registerCustomComponent(
+ 'change-view-integration', CodeOwnersPluginStatusNotifier.is)
+ .onAttached(view => {
+ view.restApi = restApi;
+ view.reporting = reporting;
+ });
+
// owner status column / rows for file list
plugin.registerDynamicCustomComponent(
'change-view-file-list-header-prepend', OwnerStatusColumnHeader.is)
@@ -61,4 +71,4 @@
view.restApi = restApi;
view.reporting = reporting;
});
-});
\ No newline at end of file
+});
diff --git a/ui/suggest-owners-trigger.js b/ui/suggest-owners-trigger.js
index 8e0e293..544c956 100644
--- a/ui/suggest-owners-trigger.js
+++ b/ui/suggest-owners-trigger.js
@@ -15,6 +15,7 @@
* limitations under the License.
*/
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {PluginState} from './code-owners-model.js';
export class SuggestOwnersTrigger extends
CodeOwnersModelMixin(Polymer.Element) {
@@ -26,7 +27,7 @@
return {
hidden: {
type: Boolean,
- computed: '_computeHidden(model.isCodeOwnerEnabled,' +
+ computed: '_computeHidden(model.pluginStatus,' +
'model.areAllFilesApproved, model.userRole)',
reflectToAttribute: true,
},
@@ -68,17 +69,17 @@
loadPropertiesAfterModelChanged() {
super.loadPropertiesAfterModelChanged();
this.modelLoader.loadUserRole();
- this.modelLoader.loadIsCodeOwnerEnabled();
+ this.modelLoader.loadPluginStatus();
this.modelLoader.loadAreAllFilesApproved();
}
- _computeHidden(enabled, allFilesApproved, userRole) {
- if (enabled === undefined ||
+ _computeHidden(pluginStatus, allFilesApproved, userRole) {
+ if (pluginStatus === undefined ||
allFilesApproved === undefined ||
userRole === undefined) {
return true;
}
- if (enabled) {
+ if (pluginStatus.state === PluginState.Enabled) {
return allFilesApproved;
} else {
return true;
diff --git a/ui/suggest-owners.js b/ui/suggest-owners.js
index 068b933..e0bf8a8 100644
--- a/ui/suggest-owners.js
+++ b/ui/suggest-owners.js
@@ -257,6 +257,7 @@
<template is="dom-if" if="[[suggestion.error]]">
<div class="fetch-error-content">
[[suggestion.error]]
+ <a on-click="_showErrorDetails"
</div>
</template>
<template is="dom-if" if="[[!suggestion.error]]">
