Improvements to error/notification process (#4053)

* Improvements to error/notification process - Show mini error message, as per create, when updating service instance - Handle all 5XX status codes as errors - Update messages to reflect varience of error states - Show single popover message on error dismiss all button - Correct error page selector * Treat calls to multiple endpoints as `global` - calls where we generate the endpoint lists (app wall, etc) were handled badly by jetstreamErrorHandler - this was due to the internal event storing endpoint id as `` instead of one per endpoint - this happened due to the list of generated endpoints being applied directly to request object in lower function - treat these as global for the moment
cloudfoundry · Dec 13, 2019 · 9d1751b · 9d1751b
1 parent d998e5c
commit 9d1751b
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 31 deletions.
diff --git a/...ed/components/add-service-instance/specify-details-step/specify-details-step.component.ts b/...ed/components/add-service-instance/specify-details-step/specify-details-step.component.ts
@@ -299,7 +299,7 @@ export class SpecifyDetailsStepComponent implements OnDestroy, AfterContentInit
           if (!!updatingInfo && updatingInfo.error) {
             return observableOf({
               success: false,
-              message: `Failed to update service instance.`
+              message: `Failed to update service instance: ${updatingInfo.message}`
             });
           }
         } else if (request.error) {

diff --git a/src/frontend/packages/core/src/app.module.ts b/src/frontend/packages/core/src/app.module.ts
@@ -21,6 +21,7 @@ import { AppStoreModule } from '../../store/src/store.module';
 import { EndpointModel } from '../../store/src/types/endpoint.types';
 import { IFavoriteMetadata, UserFavorite } from '../../store/src/types/user-favorites.types';
 import { TabNavService } from '../tab-nav.service';
+import { XSRFModule } from '../xsrf.module';
 import { AppComponent } from './app.component';
 import { RouteModule } from './app.routing';
 import { STRATOS_ENDPOINT_TYPE } from './base-entity-schemas';
@@ -46,7 +47,6 @@ import { CustomReuseStrategy } from './route-reuse-stragegy';
 import { FavoritesConfigMapper } from './shared/components/favorites-meta-card/favorite-config-mapper';
 import { endpointEventKey, GlobalEventData, GlobalEventService } from './shared/global-events.service';
 import { SharedModule } from './shared/shared.module';
-import { XSRFModule } from '../xsrf.module';
 
 // Create action for router navigation. See
 // - https://github.com/ngrx/platform/issues/68
@@ -144,7 +144,10 @@ export class AppModule {
       eventTriggered: (state: GeneralEntityAppState) => {
         const eventState = internalEventStateSelector(state);
         return Object.entries(eventState.types.endpoint).reduce((res, [eventId, value]) => {
-          const backendErrors = value.filter(error => error.eventCode === '500');
+          const backendErrors = value.filter(error => {
+            const eventCode = parseInt(error.eventCode, 10);
+            return eventCode >= 500;
+          });
           if (!backendErrors.length) {
             return res;
           }
@@ -157,7 +160,9 @@ export class AppModule {
         }, []);
       },
       message: data => {
-        return `We've been having trouble communicating with the endpoint ${data.endpoint.name}`;
+        const part1 = data.count > 1 ? `There are ${data.count} errors` : `There is an error`;
+        const part2 = data.endpoint ? ` associated with the endpoint '${data.endpoint.name}'` : ` associated with multiple endpoints`;
+        return part1 + part2;
       },
       key: data => `${endpointEventKey}-${data.endpoint.guid}`,
       link: data => `/errors/${data.endpoint.guid}`,

diff --git a/src/frontend/packages/core/src/features/error-page/error-page/error-page.component.html b/src/frontend/packages/core/src/features/error-page/error-page/error-page.component.html
@@ -17,8 +17,8 @@ <h2 class="error-page__endpoint-name">{{errorDetails.endpoint.name}}</h2>
       </div>
       <div class="error-page__header-right" *ngIf="errorDetails.errors && errorDetails.errors.length">
         <a class="error-page__download" mat-icon-button [href]="jsonDownloadHref$ | async"
-          title="Download endpoint error information" download="endpoint-errors.json"
-          matTooltip="Download endpoint error information" aria-label="Download endpoint error information">
+          download="endpoint-errors.json" matTooltip="Download endpoint error information"
+          aria-label="Download endpoint error information">
           <mat-icon>cloud_download</mat-icon>
         </a>
       </div>

diff --git a/src/frontend/packages/core/src/features/error-page/error-page/error-page.component.ts b/src/frontend/packages/core/src/features/error-page/error-page/error-page.component.ts
@@ -17,7 +17,7 @@ import { InternalEventMonitorFactory } from '../../../shared/monitors/internal-e
 import { StratosStatus } from '../../../shared/shared.types';
 
 @Component({
-  selector: 'app-events-page',
+  selector: 'app-error-page',
   templateUrl: './error-page.component.html',
   styleUrls: ['./error-page.component.scss']
 })

diff --git a/...core/src/shared/components/page-header/page-header-events/page-header-events.component.ts b/...core/src/shared/components/page-header/page-header-events/page-header-events.component.ts
@@ -16,15 +16,15 @@ import { endpointEventKey, GlobalEventService, IGlobalEvent } from '../../../glo
   animations: [
     trigger(
       'eventEnter', [
-        transition(':enter', [
-          style({ opacity: 0 }),
-          animate('250ms ease-in', style({ opacity: 1 }))
-        ]),
-        transition(':leave', [
-          style({ opacity: 1 }),
-          animate('250ms ease-out', style({ opacity: 0 }))
-        ])
-      ]
+      transition(':enter', [
+        style({ opacity: 0 }),
+        animate('250ms ease-in', style({ opacity: 1 }))
+      ]),
+      transition(':leave', [
+        style({ opacity: 1 }),
+        animate('250ms ease-out', style({ opacity: 0 }))
+      ])
+    ]
     )
   ]
 })
@@ -89,7 +89,7 @@ export class PageHeaderEventsComponent implements OnInit {
           const endpointErrorKeys = events.reduce((endpointIds, event) => {
             return endpointIds.add(this.getEndpointId(event));
           }, new Set<string>());
-          return endpointErrorKeys.size > 1 ? `We've been having trouble communicating with multiple endpoints` : events[0].message;
+          return endpointErrorKeys.size > 1 ? `There are multiple endpoints with errors` : events[0].message;
         }),
         share()
       );

diff --git a/...frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts b/...frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts
@@ -11,10 +11,11 @@ import { entityCatalogue } from '../../../core/src/core/entity-catalogue/entity-
 import { IStratosEntityDefinition } from '../../../core/src/core/entity-catalogue/entity-catalogue.types';
 import { AppState, InternalAppState } from '../app-state';
 import { PaginationFlattenerConfig } from '../helpers/paginated-request-helpers';
+import { selectPaginationState } from '../selectors/pagination.selectors';
 import { PaginatedAction, PaginationEntityState } from '../types/pagination.types';
 import {
   handleJetstreamResponsePipeFactory,
-  handleNonJetstreamResponsePipeFactory
+  handleNonJetstreamResponsePipeFactory,
 } from './entity-request-base-handlers/handle-multi-endpoints.pipe';
 import { makeRequestEntityPipe } from './entity-request-base-handlers/make-request-entity-request.pipe';
 import { mapMultiEndpointResponses } from './entity-request-base-handlers/map-multi-endpoint.pipes';
@@ -26,9 +27,8 @@ import {
 } from './entity-request-pipeline.types';
 import { getPaginationParamsPipe } from './pagination-request-base-handlers/get-params.pipe';
 import { PaginationPageIterator } from './pagination-request-base-handlers/pagination-iterator.pipe';
-import { singleRequestToPaged, isJetstreamRequest } from './pipeline-helpers';
+import { isJetstreamRequest, singleRequestToPaged } from './pipeline-helpers';
 import { PipelineHttpClient } from './pipline-http-client.service';
-import { selectPaginationState } from '../selectors/pagination.selectors';
 
 function getRequestObjectObservable(request: HttpRequest<any> | Observable<HttpRequest<any>>): Observable<HttpRequest<any>> {
   return isObservable(request) ? request : of(request);
@@ -126,7 +126,7 @@ export const basePaginatedRequestPipeline: EntityRequestPipeline = (
       ).pipe(
         // Convert { [endpointGuid]: <raw response> } to { { errors: [], successes: [] } }
         map(handleMultiEndpointsPipe),
-        // Convert { { errors: [], successes: [] } } to { response: NoramlisedResponse, success: boolean }
+        // Convert { { errors: [], successes: [] } } to { response: NormalisedResponse, success: boolean }
         map(multiEndpointResponses => mapMultiEndpointResponses(
           completePaginationAction,
           catalogueEntity,

diff --git a/...store/src/entity-request-pipeline/entity-request-base-handlers/jetstream-error.handler.ts b/...store/src/entity-request-pipeline/entity-request-base-handlers/jetstream-error.handler.ts
@@ -3,7 +3,7 @@ import { SendEventAction } from '../../actions/internal-events.actions';
 import { RecursiveDeleteFailed } from '../../effects/recursive-entity-delete.effect';
 import { endpointSchemaKey } from '../../helpers/entity-factory';
 import { ApiRequestTypes, getFailApiRequestActions } from '../../reducers/api-request-reducer/request-helpers';
-import { InternalEventSeverity, InternalEventStateMetadata } from '../../types/internal-events.types';
+import { GLOBAL_EVENT, InternalEventSeverity, InternalEventStateMetadata } from '../../types/internal-events.types';
 import { EntityRequestAction } from '../../types/request.types';
 import { ActionDispatcher } from '../entity-request-pipeline.types';
 import { PipelineHttpClient } from '../pipline-http-client.service';
@@ -17,11 +17,30 @@ export function jetstreamErrorHandler(
   actionDispatcher: ActionDispatcher,
   recursivelyDeleting: boolean
 ) {
-  const endpointString = action.options.headers ? action.options.headers.get(PipelineHttpClient.EndpointHeader) || '' : '';
-  const endpointIds: string[] = endpointString.split(',');
-  endpointIds.forEach(endpoint =>
+  // This will never work for calls where endpoint list is automatically generated (list is applied to request object not action)
+  // For those cases treat as a global error
+  const endpointString = action.options.headers ? action.options.headers.get(PipelineHttpClient.EndpointHeader) || null : null;
+  const endpointIds: string[] = endpointString ? endpointString.split(',') : [];
+
+  if (endpointString) {
+    endpointIds.forEach(endpoint =>
+      actionDispatcher(
+        new SendEventAction<InternalEventStateMetadata>(endpointSchemaKey, endpoint, {
+          eventCode: error.status ? error.status + '' : '500',
+          severity: InternalEventSeverity.ERROR,
+          message: 'Jetstream API request error',
+          metadata: {
+            httpMethod: action.options.method as string,
+            errorResponse: error,
+            url: error.url || action.options.url,
+          },
+        }),
+      ),
+    );
+  } else {
+    // See #4054, in theory we should never hit this as we always know the endpoint id's
     actionDispatcher(
-      new SendEventAction<InternalEventStateMetadata>(endpointSchemaKey, endpoint, {
+      new SendEventAction<InternalEventStateMetadata>(GLOBAL_EVENT, catalogueEntity.entityKey, {
         eventCode: error.status ? error.status + '' : '500',
         severity: InternalEventSeverity.ERROR,
         message: 'Jetstream API request error',
@@ -31,8 +50,9 @@ export function jetstreamErrorHandler(
           url: error.url || action.options.url,
         },
       }),
-    ),
-  );
+    );
+  }
+
   const errorActions = getFailApiRequestActions(action, error, requestType, catalogueEntity, {
     endpointIds,
     url: error.url || action.options.url,

diff --git a/.../entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts b/.../entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts
@@ -8,7 +8,7 @@ export const makeRequestEntityPipe: MakeEntityRequestPipe = (
   httpClient,
   requestOrObservable,
   endpointConfig: StratosCatalogueEndpointEntity,
-  endpointGuids: string[],
+  endpointGuids: string | string[],
   externalRequest: boolean = false
 ) => {
   if (requestOrObservable instanceof HttpRequest) {

diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts
@@ -78,11 +78,11 @@ export const apiRequestPipelineFactory = (
     map(() => catalogueEntity.getRequestAction('complete', action, requestType)),
     catchError(error => {
       const httpResponse = isHttpErrorResponse(error);
-      const res: PipelineResult = {
+      const response: PipelineResult = {
         success: false,
         errorMessage: httpResponse ? httpResponse.error : null
       };
-      failedEntityHandler(actionDispatcher, catalogueEntity, requestType, action, res, recursivelyDelete);
+      failedEntityHandler(actionDispatcher, catalogueEntity, requestType, action, response, recursivelyDelete);
       jetstreamErrorHandler(
         error,
         patchedAction,