You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ol...@apache.org on 2018/04/09 14:03:50 UTC

[ambari] branch trunk updated: [AMBARI-23509] Fix the unhandled http request promise error (#932)

This is an automated email from the ASF dual-hosted git repository.

oleewere pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new e3b3971  [AMBARI-23509] Fix the unhandled http request promise error (#932)
e3b3971 is described below

commit e3b3971e35d055a05bcf541dc42cf4e17417794b
Author: Istvan Tobias <to...@gmail.com>
AuthorDate: Mon Apr 9 16:03:46 2018 +0200

    [AMBARI-23509] Fix the unhandled http request promise error (#932)
---
 .../ambari-logsearch-web/src/app/app.module.ts     |  4 +--
 .../login-form/login-form.component.html           |  2 +-
 .../components/login-form/login-form.component.ts  | 20 +++---------
 .../modules/app-load/services/app-load.service.ts  | 29 +++++++++--------
 .../src/app/services/auth.service.ts               | 38 +++++++++++-----------
 .../src/app/services/http-client.service.ts        | 20 +++++++-----
 .../src/app/services/mock-api-data.service.spec.ts | 14 ++++----
 .../src/app/services/mock-api-data.service.ts      | 24 +++++++-------
 .../src/app/test-config.spec.ts                    |  4 +--
 9 files changed, 74 insertions(+), 81 deletions(-)

diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/app.module.ts b/ambari-logsearch/ambari-logsearch-web/src/app/app.module.ts
index 440a6ee..b0612fe 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/app.module.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/app.module.ts
@@ -38,7 +38,7 @@ import {ShipperModule} from '@modules/shipper/shipper.module';
 
 import {ServiceInjector} from '@app/classes/service-injector';
 
-import {mockApiDataService} from '@app/services/mock-api-data.service';
+import {MockApiDataService} from '@app/services/mock-api-data.service';
 import {HttpClientService} from '@app/services/http-client.service';
 import {UtilsService} from '@app/services/utils.service';
 import {LogsContainerService} from '@app/services/logs-container.service';
@@ -123,7 +123,7 @@ export function getXHRBackend(
   } else {
     return new InMemoryBackendService(
       injector,
-      new mockApiDataService(),
+      new MockApiDataService(),
       {
         passThruUnknownUrl: true,
         rootPath: ''
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.html b/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.html
index a34cfb8..8b9c957 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.html
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.html
@@ -26,7 +26,7 @@
       <label for="password">{{'authorization.password' | translate}}</label>
       <input class="form-control" type="password" id="password" name="password" required [(ngModel)]="password">
     </div>
-    <button class="btn btn-success" [disabled]="!loginForm.form.valid || isLoginInProgress">
+    <button class="btn btn-success" [disabled]="!loginForm.form.valid || (isLoginInProgress$ | async)">
       {{'authorization.signIn' | translate}}
     </button>
   </form>
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.ts b/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.ts
index 03403b7..e3570c4 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/components/login-form/login-form.component.ts
@@ -16,21 +16,19 @@
  * limitations under the License.
  */
 
-import {Component, OnDestroy, OnInit} from '@angular/core';
+import {Component} from '@angular/core';
 import {Response} from '@angular/http';
 import 'rxjs/add/operator/finally';
 import {AppStateService} from '@app/services/storage/app-state.service';
 import {AuthService} from '@app/services/auth.service';
-import {Subscription} from "rxjs/Subscription";
+import {Observable} from 'rxjs/Observable';
 
 @Component({
   selector: 'login-form',
   templateUrl: './login-form.component.html',
   styleUrls: ['./login-form.component.less']
 })
-export class LoginFormComponent implements OnInit, OnDestroy {
-
-  private subscriptions: Subscription[] = [];
+export class LoginFormComponent {
 
   username: string;
 
@@ -38,20 +36,10 @@ export class LoginFormComponent implements OnInit, OnDestroy {
 
   isLoginAlertDisplayed: boolean;
 
-  isLoginInProgress: boolean;
+  isLoginInProgress$: Observable<boolean> = this.appState.getParameter('isLoginInProgress');
 
   constructor(private authService: AuthService, private appState: AppStateService) {}
 
-  ngOnInit() {
-    this.subscriptions.push(
-      this.appState.getParameter('isLoginInProgress').subscribe(value => this.isLoginInProgress = value)
-    );
-  }
-
-  ngOnDestroy() {
-    this.subscriptions.forEach((subscription: Subscription) => subscription.unsubscribe());
-  }
-
   /**
    * Handling the response from the login action. Actually the goal only to show or hide the login error alert.
    * When it gets error response it shows.
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/modules/app-load/services/app-load.service.ts b/ambari-logsearch/ambari-logsearch-web/src/app/modules/app-load/services/app-load.service.ts
index 7bcedfb..3a207be 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/modules/app-load/services/app-load.service.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/modules/app-load/services/app-load.service.ts
@@ -53,7 +53,7 @@ export class AppLoadService {
   loadClusters(): Observable<Response> {
     this.clustersStorage.clear();
     this.appStateService.setParameter('clustersDataState', DataAvailabilityValues.LOADING);
-    const response$: Observable<Response> = this.httpClient.get('clusters').first();
+    const response$: Observable<Response> = this.httpClient.get('clusters').filter((response: Response) => response.ok).first();
     response$.subscribe(
       (response: Response) => {
         const clusterNames = response.json();
@@ -77,7 +77,7 @@ export class AppLoadService {
   loadHosts(): Observable<Response> {
     this.hostStoreService.clear();
     this.appStateService.setParameter('hostsDataState', DataAvailabilityValues.LOADING);
-    const response$ = this.httpClient.get('hosts');
+    const response$ = this.httpClient.get('hosts').filter((response: Response) => response.ok);
     response$.subscribe((response: Response): void => {
       const jsonResponse = response.json(),
         hosts = jsonResponse && jsonResponse.vNodeList;
@@ -99,12 +99,14 @@ export class AppLoadService {
   loadComponents(): Observable<[{[key: string]: any}, {[key: string]: any}]> {
     this.appStateService.setParameter('componentsDataState', DataAvailabilityValues.LOADING);
     const responseComponentsData$: Observable<Response> = this.httpClient.get('components').first()
+      .filter((response: Response) => response.ok)
       .map((response: Response) => response.json());
     const responseComponentsName$: Observable<Response> = this.httpClient.get('serviceComponentsName').first()
+      .filter((response: Response) => response.ok)
       .map((response: Response) => response.json());
     const responses$ = Observable.combineLatest(responseComponentsName$, responseComponentsData$);
     responses$.subscribe(([componentsNames, componentsData]: [{[key: string]: any}, {[key: string]: any}]) => {
-      const components = componentsData && componentsData.vNodeList.map((item): NodeItem => {
+      const components = componentsData && componentsData.vNodeList && componentsData.vNodeList.map((item): NodeItem => {
         const component = componentsNames.metadata.find(componentItem => componentItem.name === item.name);
         return Object.assign(item, {
           label: component && (component.label || item.name),
@@ -131,10 +133,12 @@ export class AppLoadService {
 
   loadFieldsForLogs(): Observable<[LogField[], AuditFieldsDefinitionSet]> {
     const serviceLogsFieldsResponse$: Observable<LogField[]> = this.httpClient.get('serviceLogsFields')
+      .filter((response: Response) => response.ok)
       .map((response: Response) => {
         return response.json();
       });
     const auditLogsFieldsResponse$: Observable<AuditFieldsDefinitionSet> = this.httpClient.get('auditLogsFields')
+      .filter((response: Response) => response.ok)
       .map((response: Response) => {
         return response.json();
       });
@@ -166,18 +170,15 @@ export class AppLoadService {
   }
 
   syncAuthorizedStateWithBackend(): Promise<any> {
-    const statusRequest: Promise<Response> = this.httpClient.get('status').toPromise();
-    statusRequest.then(
-      () => this.appStateService.setParameters({
-        isAuthorized: true,
+    const setAuthorization = (isAuthorized: boolean) => {
+      this.appStateService.setParameters({
+        isAuthorized,
         isInitialLoading: false
-      }),
-      () => this.appStateService.setParameters({
-        isAuthorized: false,
-        isInitialLoading: false
-      })
-    );
-    return statusRequest;
+      });
+    };
+    const statusRequestPromise: Promise<Response> = this.httpClient.get('status').toPromise();
+    statusRequestPromise.then((response: Response) => setAuthorization(response.ok));
+    return statusRequestPromise;
   }
 
   setTranslationService() {
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/services/auth.service.ts b/ambari-logsearch/ambari-logsearch-web/src/app/services/auth.service.ts
index f63ebd8..2880aa7 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/services/auth.service.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/services/auth.service.ts
@@ -35,17 +35,7 @@ export const IS_LOGIN_IN_PROGRESS_APP_STATE_KEY: string = 'isLoginInProgress';
 @Injectable()
 export class AuthService {
 
-  constructor(
-    private httpClient: HttpClientService,
-    private appState: AppStateService,
-    private router: Router
-  ) {
-    this.appStateIsAuthorizedSubscription = this.appState.getParameter(IS_AUTHORIZED_APP_STATE_KEY).subscribe(
-      this.onAppStateIsAuthorizedChanged
-    );
-  }
-
-  private appStateIsAuthorizedSubscription: Subscription;
+  private subscriptions: Subscription[] = [];
 
   /**
    * A string set by any service or component (mainly from AuthGuard service) to redirect the application after the
@@ -54,12 +44,22 @@ export class AuthService {
    */
   redirectUrl: string;
 
+  constructor(
+    private httpClient: HttpClientService,
+    private appState: AppStateService,
+    private router: Router
+  ) {
+    this.subscriptions.push(this.appState.getParameter(IS_AUTHORIZED_APP_STATE_KEY).subscribe(
+      this.onAppStateIsAuthorizedChanged
+    ));
+  }
+
   onAppStateIsAuthorizedChanged = (isAuthorized):void => {
     if (isAuthorized && this.redirectUrl) {
       this.router.navigate([this.redirectUrl]);
       this.redirectUrl = '';
     }
-  };
+  }
   /**
    * The single entry point to request a login action.
    * @param {string} username
@@ -68,15 +68,15 @@ export class AuthService {
    */
   login(username: string, password: string): Observable<Response> {
     this.setLoginInProgressAppState(true);
-    let obs = this.httpClient.postFormData('login', {
+    const response$ = this.httpClient.postFormData('login', {
       username: username,
       password: password
     });
-    obs.subscribe(
+    response$.subscribe(
       (resp: Response) => this.onLoginResponse(resp),
       (resp: Response) => this.onLoginError(resp)
     );
-    return obs;
+    return response$;
   }
 
   /**
@@ -84,12 +84,12 @@ export class AuthService {
    * @returns {Observable<boolean | Error>}
    */
   logout(): Observable<Response> {
-    let obs = this.httpClient.get('logout');
-    obs.subscribe(
+    const response$ = this.httpClient.get('logout');
+    response$.subscribe(
       (resp: Response) => this.onLogoutResponse(resp),
       (resp: Response) => this.onLogoutError(resp)
     );
-    return obs;
+    return response$;
   }
 
   /**
@@ -115,8 +115,8 @@ export class AuthService {
    * @param resp
    */
   private onLoginResponse(resp: Response): void {
+    this.setLoginInProgressAppState(false);
     if (resp && resp.ok) {
-      this.setLoginInProgressAppState(false);
       this.setAuthorizedAppState(resp.ok);
     }
   }
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/services/http-client.service.ts b/ambari-logsearch/ambari-logsearch-web/src/app/services/http-client.service.ts
index 3d49efd..9d186b2 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/services/http-client.service.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/services/http-client.service.ts
@@ -19,6 +19,7 @@
 import {Injectable} from '@angular/core';
 import {Observable} from 'rxjs/Observable';
 import 'rxjs/add/operator/first';
+import 'rxjs/add/observable/throw';
 import {
   Http, XHRBackend, Request, RequestOptions, RequestOptionsArgs, Response, Headers, URLSearchParams
 } from '@angular/http';
@@ -150,17 +151,20 @@ export class HttpClientService extends Http {
     }
   }
 
-  private handleError(request: Observable<Response>): void {
-    request.subscribe(null, (error: any) => {
+  request(url: string | Request, options?: RequestOptionsArgs): Observable<Response> {
+    const handleResponseError = (error) => {
+      let handled: boolean = false;
       if (this.unauthorizedStatuses.indexOf(error.status) > -1) {
         this.appState.setParameter('isAuthorized', false);
+        handled = true;
       }
-    });
-  }
-
-  request(url: string | Request, options?: RequestOptionsArgs): Observable<Response> {
-    const req: Observable<Response> = super.request(this.generateUrl(url), options).share().first();
-    this.handleError(req);
+      return handled;
+    };
+    const req: Observable<Response> = super.request(this.generateUrl(url), options).share()
+      .map(response => response)
+      .catch((error: any) => {
+        return handleResponseError(error) ? Observable.of(error) : Observable.throw(error);
+      });
     return req;
   }
 
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.spec.ts b/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.spec.ts
index 410f70d..5f6a2e1 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.spec.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.spec.ts
@@ -17,16 +17,16 @@
  */
 
 import {TestBed, inject} from '@angular/core/testing';
-import {mockApiDataService} from './mock-api-data.service';
+import {MockApiDataService} from './mock-api-data.service';
 
-describe('mockApiDataService', () => {
+describe('MockApiDataService', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
-      providers: [mockApiDataService]
+      providers: [MockApiDataService]
     });
   });
 
-  it('should create service', inject([mockApiDataService], (service: mockApiDataService) => {
+  it('should create service', inject([MockApiDataService], (service: MockApiDataService) => {
     expect(service).toBeTruthy();
   }));
 
@@ -64,15 +64,15 @@ describe('mockApiDataService', () => {
 
     cases.forEach(test => {
       describe(test.title, () => {
-        it('base', inject([mockApiDataService], (service: mockApiDataService) => {
+        it('base', inject([MockApiDataService], (service: MockApiDataService) => {
           expect(service.parseUrl(test.url).base).toEqual(test.base);
         }));
 
-        it('collectionName', inject([mockApiDataService], (service: mockApiDataService) => {
+        it('collectionName', inject([MockApiDataService], (service: MockApiDataService) => {
           expect(service.parseUrl(test.url).collectionName).toEqual(test.collectionName);
         }));
 
-        it('query', inject([mockApiDataService], (service: mockApiDataService) => {
+        it('query', inject([MockApiDataService], (service: MockApiDataService) => {
           expect(service.parseUrl(test.url).query.toString()).toEqual(test.query);
         }));
       });
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.ts b/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.ts
index 15af7cf..c5c437d 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/services/mock-api-data.service.ts
@@ -25,25 +25,25 @@ import * as moment from 'moment';
 import {mockDataGet} from "@mockdata/mock-data-get";
 import {mockDataPost} from "@mockdata/mock-data-post";
 
-export class mockBackendService extends InMemoryBackendService {
+export class MockBackendService extends InMemoryBackendService {
   getLocation(url: string): any {
     return super.getLocation(url);
   }
 }
 
-export class mockApiDataService implements InMemoryDbService {
+export class MockApiDataService implements InMemoryDbService {
 
   private filterByMessage = (value: string, filterValue: string): boolean => {
     return value.toLowerCase().indexOf(filterValue.toLowerCase()) > -1;
-  };
+  }
 
   private filterByStartTime = (value: number, filterValue: number | string | Date | moment.Moment): boolean => {
     return value >= moment(filterValue).valueOf();
-  };
+  }
 
   private filterByEndTime = (value: number, filterValue: number | string | Date | moment.Moment): boolean => {
     return value <= moment(filterValue).valueOf();
-  };
+  }
 
   private readonly filterMap = {
     'api/v1/service/logs': {
@@ -109,7 +109,7 @@ export class mockApiDataService implements InMemoryDbService {
   };
 
   parseUrl(url: string): any {
-    const urlLocation = mockBackendService.prototype.getLocation(url),
+    const urlLocation = MockBackendService.prototype.getLocation(url),
       query = urlLocation.search && new URLSearchParams(urlLocation.search.substr(1), {
           encodeKey: key => key,
           encodeValue: value => value
@@ -127,8 +127,8 @@ export class mockApiDataService implements InMemoryDbService {
 
   private findDataByUrlPatter(path: string, mockDataObj: {[key: string]: any}): {[key: string]: any} | undefined | Function {
     const paths: string[] = Object.keys(mockDataObj);
-    const matchedPath:string = paths.find((key:string):boolean => {
-      const test:RegExp = new RegExp(key);
+    const matchedPath: string = paths.find((key: string): boolean => {
+      const test: RegExp = new RegExp(key);
       return test.test(path);
     });
     return mockDataObj[matchedPath];
@@ -147,7 +147,7 @@ export class mockApiDataService implements InMemoryDbService {
       if (typeof allData === 'function') {
         allData = allData(query, interceptorArgs.requestInfo.req);
       }
-      let is404 = !allData;
+      const is404 = !allData;
 
       if (is404) {
         return new Observable<Response>((subscriber: Subscriber<Response>) => subscriber.error(
@@ -200,8 +200,8 @@ export class mockApiDataService implements InMemoryDbService {
             filteredData[filterMapItem.totalCountKey] = filteredCollection.length;
           }
           if (query && query.paramsMap.has('page') && query.paramsMap.has('pageSize')) {
-            const page = parseInt(query.paramsMap.get('page')[0]),
-              pageSize = parseInt(query.paramsMap.get('pageSize')[0]);
+            const page = parseInt(query.paramsMap.get('page')[0], 0),
+              pageSize = parseInt(query.paramsMap.get('pageSize')[0], 0);
             filteredCollection = filteredCollection.slice(page * pageSize, (page + 1) * pageSize);
           }
           filteredData[pathToCollection] = filteredCollection;
@@ -231,7 +231,7 @@ export class mockApiDataService implements InMemoryDbService {
     if (typeof responseBody === 'function') {
       responseBody = responseBody(query, interceptorArgs.requestInfo.req);
     }
-    let is404 = !responseBody;
+    const is404 = !responseBody;
 
     if (is404) {
       return new Observable<Response>((subscriber: Subscriber<Response>) => subscriber.error(
diff --git a/ambari-logsearch/ambari-logsearch-web/src/app/test-config.spec.ts b/ambari-logsearch/ambari-logsearch-web/src/app/test-config.spec.ts
index 18faf8c..9a53a37 100644
--- a/ambari-logsearch/ambari-logsearch-web/src/app/test-config.spec.ts
+++ b/ambari-logsearch/ambari-logsearch-web/src/app/test-config.spec.ts
@@ -21,7 +21,7 @@ import {TranslateModule, TranslateLoader} from '@ngx-translate/core';
 import {TranslateHttpLoader} from '@ngx-translate/http-loader';
 import {Injector} from '@angular/core';
 import {InMemoryBackendService} from 'angular-in-memory-web-api';
-import {mockApiDataService} from '@app/services/mock-api-data.service';
+import {MockApiDataService} from '@app/services/mock-api-data.service';
 import {HttpClientService} from '@app/services/http-client.service';
 import {RouterTestingModule} from '@angular/router/testing';
 import {clusters, ClustersService} from '@app/services/storage/clusters.service';
@@ -83,7 +83,7 @@ export const getCommonTestingBedConfiguration = (
 export function getTestXHRBackend(injector: Injector, browser: BrowserXhr, xsrf: XSRFStrategy, options: ResponseOptions) {
   return new InMemoryBackendService(
     injector,
-    new mockApiDataService(),
+    new MockApiDataService(),
     {
       passThruUnknownUrl: true,
       rootPath: ''

-- 
To stop receiving notification emails like this one, please contact
oleewere@apache.org.