Fix: Fixes user logout for datasourceRequests with 401 from datasource (#21800)

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<FetchResponse> => 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<any>) =>
+        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);

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/';
+        await backendSrv
-        // it would be better if we could simulate that after the call to loginPing everything is successful but as
+          .request({ url, method: 'GET', retry: 0 })
-        // our fromFetchMock returns ok:false the second time this retries it will still be ok:false going into the
+          .catch(error => {
-        // mergeMap in toFailureStream
+            expect(error.status).toBe(401);
-        await backendSrv.request({ url, method: 'GET', retry: 0 }).catch(error => {
+            expect(error.statusText).toBe('UnAuthorized');
-          expect(error.status).toBe(401);
+            expect(error.data).toEqual({ message: 'UnAuthorized' });
-          expect(error.statusText).toBe('UnAuthorized');
+            expect(appEventsMock.emit).not.toHaveBeenCalled();
-          expect(error.data).toEqual({ message: 'UnAuthorized' });
+            expect(logoutMock).not.toHaveBeenCalled();
-          expect(appEventsMock.emit).not.toHaveBeenCalled();
+            expect(backendSrv.loginPing).toHaveBeenCalledTimes(1);
-          expect(backendSrv.loginPing).toHaveBeenCalledTimes(1);
+            expectRequestCallChain({ url, method: 'GET', retry: 0 });
-          expect(logoutMock).toHaveBeenCalledTimes(1);
+            jest.advanceTimersByTime(50);
-          expectRequestCallChain({ url, method: 'GET', retry: 0 });
+          })
-          jest.advanceTimersByTime(50);
+          .catch(error => {
-          expect(appEventsMock.emit).not.toHaveBeenCalled();
+            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 });
         });
       });