diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index eac77a65cc9..b7a92d9abbe 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -170,11 +170,6 @@ export class BackendSrv implements BackendService { return merge(successStream, failureStream) .pipe( catchError((err: ErrorResponse) => { - if (err.status === 401) { - this.dependencies.logout(); - return throwError(err); - } - // this setTimeout hack enables any caller catching this err to set isHandled to true setTimeout(() => this.requestErrorHandler(err), 50); return throwError(err); @@ -202,7 +197,7 @@ export class BackendSrv implements BackendService { ); const fromFetchStream = this.getFromFetchStream(options); - const failureStream = fromFetchStream.pipe(this.toFailureStream(options)); + const failureStream = fromFetchStream.pipe(this.toDataSourceRequestFailureStream(options)); const successStream = fromFetchStream.pipe( filter(response => response.ok === true), map(response => { @@ -226,11 +221,6 @@ export class BackendSrv implements BackendService { }); } - if (err.status === 401) { - this.dependencies.logout(); - return throwError(err); - } - // populate error obj on Internal Error if (typeof err.data === 'string' && err.status === 500) { err.data = { @@ -498,7 +488,50 @@ export class BackendSrv implements BackendService { const firstAttempt = i === 0 && options.retry === 0; if (error.status === 401 && this.dependencies.contextSrv.user.isSignedIn && firstAttempt) { - return from(this.loginPing()); + return from(this.loginPing()).pipe( + catchError(err => { + if (err.status === 401) { + this.dependencies.logout(); + return throwError(err); + } + return throwError(err); + }) + ); + } + + return throwError(error); + }) + ) + ) + ); + + private toDataSourceRequestFailureStream = ( + options: BackendSrvRequest + ): MonoTypeOperatorFunction => inputStream => + inputStream.pipe( + filter(response => response.ok === false), + mergeMap(response => { + const { status, statusText, data } = response; + const fetchErrorResponse: ErrorResponse = { status, statusText, data }; + return throwError(fetchErrorResponse); + }), + retryWhen((attempts: Observable) => + attempts.pipe( + mergeMap((error, i) => { + const requestIsLocal = !options.url.match(/^http/); + const firstAttempt = i === 0 && options.retry === 0; + + // First retry, if loginPing returns 401 this retry sequence will abort with throwError and user is logged out + if (requestIsLocal && firstAttempt && error.status === 401) { + return from(this.loginPing()).pipe( + catchError(err => { + if (err.status === 401) { + this.dependencies.logout(); + return throwError(err); + } + return throwError(err); + }) + ); } return throwError(error); diff --git a/public/app/core/specs/backend_srv.test.ts b/public/app/core/specs/backend_srv.test.ts index 2157daef63f..7c22c0021f1 100644 --- a/public/app/core/specs/backend_srv.test.ts +++ b/public/app/core/specs/backend_srv.test.ts @@ -164,30 +164,34 @@ describe('backendSrv', () => { describe('when making an unsuccessful call and conditions for retry are favorable and loginPing does not throw', () => { it('then it should retry', async () => { jest.useFakeTimers(); + const url = '/api/dashboard/'; const { backendSrv, appEventsMock, logoutMock, expectRequestCallChain } = getTestContext({ ok: false, status: 401, statusText: 'UnAuthorized', data: { message: 'UnAuthorized' }, + url, }); backendSrv.loginPing = jest .fn() .mockResolvedValue({ ok: true, status: 200, statusText: 'OK', data: { message: 'Ok' } }); - const url = '/api/dashboard/'; - // it would be better if we could simulate that after the call to loginPing everything is successful but as - // our fromFetchMock returns ok:false the second time this retries it will still be ok:false going into the - // mergeMap in toFailureStream - await backendSrv.request({ url, method: 'GET', retry: 0 }).catch(error => { - expect(error.status).toBe(401); - expect(error.statusText).toBe('UnAuthorized'); - expect(error.data).toEqual({ message: 'UnAuthorized' }); - expect(appEventsMock.emit).not.toHaveBeenCalled(); - expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); - expect(logoutMock).toHaveBeenCalledTimes(1); - expectRequestCallChain({ url, method: 'GET', retry: 0 }); - jest.advanceTimersByTime(50); - expect(appEventsMock.emit).not.toHaveBeenCalled(); - }); + await backendSrv + .request({ url, method: 'GET', retry: 0 }) + .catch(error => { + expect(error.status).toBe(401); + expect(error.statusText).toBe('UnAuthorized'); + expect(error.data).toEqual({ message: 'UnAuthorized' }); + expect(appEventsMock.emit).not.toHaveBeenCalled(); + expect(logoutMock).not.toHaveBeenCalled(); + expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); + expectRequestCallChain({ url, method: 'GET', retry: 0 }); + jest.advanceTimersByTime(50); + }) + .catch(error => { + expect(error).toEqual({ message: 'UnAuthorized' }); + expect(appEventsMock.emit).toHaveBeenCalledTimes(1); + expect(appEventsMock.emit).toHaveBeenCalledWith(AppEvents.alertWarning, ['UnAuthorized', '']); + }); }); }); @@ -430,16 +434,18 @@ describe('backendSrv', () => { .fn() .mockResolvedValue({ ok: true, status: 200, statusText: 'OK', data: { message: 'Ok' } }); const url = '/api/dashboard/'; - // it would be better if we could simulate that after the call to loginPing everything is successful but as - // our fromFetchMock returns ok:false the second time this retries it will still be ok:false going into the - // mergeMap in toFailureStream await backendSrv.datasourceRequest({ url, method: 'GET', retry: 0 }).catch(error => { expect(error.status).toBe(401); expect(error.statusText).toBe('UnAuthorized'); expect(error.data).toEqual({ message: 'UnAuthorized' }); - expect(appEventsMock.emit).not.toHaveBeenCalled(); + expect(appEventsMock.emit).toHaveBeenCalledTimes(1); + expect(appEventsMock.emit).toHaveBeenCalledWith(CoreEvents.dsRequestError, { + data: { message: 'UnAuthorized' }, + status: 401, + statusText: 'UnAuthorized', + }); expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); - expect(logoutMock).toHaveBeenCalledTimes(1); + expect(logoutMock).not.toHaveBeenCalled(); expectDataSourceRequestCallChain({ url, method: 'GET', retry: 0 }); }); });