Stop constant reloading of BrowserViews for servers that are not available (#2980)

src/main/server/serverAPI.test.js

@@ -8,7 +8,6 @@ import {net, session} from 'electron';
 import {getServerAPI} from './serverAPI';
 
 const validURL = 'http://server-1.com/api/endpoint';
-const badDataURL = 'http://server-1.com/api/bad/endpoint';
 const testData = {
     name: 'some data',
     value: 'some more data',
@@ -21,13 +20,13 @@ jest.mock('electron', () => ({
                 requestCallback({
                     on: jest.fn().mockImplementation((event, responseCallback) => {
                         if (event === 'data') {
-                            responseCallback(url === badDataURL ? '98&H09986t&(*6BV789RhN^t97rb6Ev^*e5v89 re5bg^&' : JSON.stringify(testData));
+                            responseCallback(JSON.stringify(testData));
                         }
                         if (event === 'end') {
                             responseCallback();
                         }
                     }),
-                    statusCode: (url === validURL || url === badDataURL) ? 200 : 404,
+                    statusCode: url === validURL ? 200 : 404,
                 });
             }),
             end: jest.fn(),
@@ -50,7 +49,7 @@ describe('main/server/serverAPI', () => {
             false,
             successFn,
         );
-        expect(successFn).toHaveBeenCalledWith(testData);
+        expect(successFn).toHaveBeenCalledWith(JSON.stringify(testData));
     });
 
     it('should call onError when bad status code received', async () => {
@@ -67,20 +66,6 @@ describe('main/server/serverAPI', () => {
         expect(errorFn).toHaveBeenCalled();
     });
 
-    it('should call onError when data parsing fails', async () => {
-        const successFn = jest.fn();
-        const errorFn = jest.fn();
-        await getServerAPI(
-            badDataURL,
-            false,
-            successFn,
-            null,
-            errorFn,
-        );
-        expect(successFn).not.toHaveBeenCalled();
-        expect(errorFn).toHaveBeenCalled();
-    });
-
     it('should call onError when response encounters an error', async () => {
         net.request.mockImplementation(({url}) => ({
             on: jest.fn().mockImplementation((_, requestCallback) => {
@@ -90,7 +75,7 @@ describe('main/server/serverAPI', () => {
                             responseCallback();
                         }
                     }),
-                    statusCode: (url === validURL || url === badDataURL) ? 200 : 404,
+                    statusCode: url === validURL ? 200 : 404,
                 });
             }),
             end: jest.fn(),

src/main/server/serverAPI.ts

@@ -8,7 +8,7 @@ import {Logger} from 'common/log';
 
 const log = new Logger('serverAPI');
 
-export async function getServerAPI<T>(url: URL, isAuthenticated: boolean, onSuccess?: (data: T) => void, onAbort?: () => void, onError?: (error: Error) => void) {
+export async function getServerAPI(url: URL, isAuthenticated: boolean, onSuccess?: (raw: string) => void, onAbort?: () => void, onError?: (error: Error) => void) {
     if (isAuthenticated) {
         const cookies = await session.defaultSession.cookies.get({});
         if (!cookies) {
@@ -47,8 +47,7 @@ export async function getServerAPI<T>(url: URL, isAuthenticated: boolean, onSucc
                 });
                 response.on('end', () => {
                     try {
-                        const data = JSON.parse(raw) as T;
+                        onSuccess(raw);
-                        onSuccess(data);
                     } catch (e) {
                         const error = `Error parsing server data from ${url.toString()}`;
                         log.error(error);

src/main/server/serverInfo.test.js

@@ -0,0 +1,54 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import {getServerAPI} from './serverAPI';
+import {ServerInfo} from './serverInfo';
+
+jest.mock('./serverAPI', () => ({
+    getServerAPI: jest.fn(),
+}));
+
+describe('main/server/serverInfo', () => {
+    describe('getRemoteInfo', () => {
+        const serverInfo = new ServerInfo({url: 'http://someurl.com'});
+        const testData = {some: 'test'};
+        const testString = JSON.stringify(testData);
+        const callback = jest.fn();
+
+        beforeEach(() => {
+            getServerAPI.mockImplementation((url, auth, success) => {
+                success(testString);
+            });
+        });
+
+        afterEach(() => {
+            callback.mockClear();
+        });
+
+        it('should return success callback when data is successfully parsed', async () => {
+            await serverInfo.getRemoteInfo(callback, '/some/url');
+            expect(callback).toHaveBeenCalledWith(testData);
+        });
+
+        it('should reject when URL is missing', async () => {
+            await expect(serverInfo.getRemoteInfo(callback)).rejects.toThrow();
+            expect(callback).not.toHaveBeenCalled();
+        });
+
+        it('should reject when JSON does not parse', async () => {
+            getServerAPI.mockImplementation((url, auth, success) => {
+                success('T^&V*RT^&*%BDF*H^(YTNB*&F&^TB(FC');
+            });
+            await expect(serverInfo.getRemoteInfo(callback)).rejects.toThrow();
+            expect(callback).not.toHaveBeenCalled();
+        });
+
+        it('should reject when request fails', async () => {
+            getServerAPI.mockImplementation((url, auth, success, abort, fail) => {
+                fail();
+            });
+            await expect(serverInfo.getRemoteInfo(callback)).rejects.toThrow();
+            expect(callback).not.toHaveBeenCalled();
+        });
+    });
+});

src/main/server/serverInfo.ts

@@ -44,12 +44,17 @@ export class ServerInfo {
             return Promise.reject(new Error('Malformed URL'));
         }
         return new Promise<void>((resolve, reject) => {
-            getServerAPI<T>(
+            getServerAPI(
                 url,
                 false,
-                (data: T) => {
+                (raw: string) => {
-                    callback(data);
+                    try {
-                    resolve();
+                        const data = JSON.parse(raw) as T;
+                        callback(data);
+                        resolve();
+                    } catch (e) {
+                        reject(e);
+                    }
                 },
                 () => reject(new Error('Aborted')),
                 (error: Error) => reject(error));

src/main/views/MattermostBrowserView.ts

@@ -24,6 +24,7 @@ import ServerManager from 'common/servers/serverManager';
 import {RELOAD_INTERVAL, MAX_SERVER_RETRIES, SECOND, MAX_LOADING_SCREEN_SECONDS} from 'common/utils/constants';
 import {isInternalURL, parseURL} from 'common/utils/url';
 import type {MattermostView} from 'common/views/View';
+import {getServerAPI} from 'main/server/serverAPI';
 import MainWindow from 'main/windows/mainWindow';
 
 import WebContentsEventManager from './webContentEvents';
@@ -235,10 +236,10 @@ export class MattermostBrowserView extends EventEmitter {
         }
     };
 
-    reload = () => {
+    reload = (loadURL?: URL | string) => {
         this.resetLoadingStatus();
         AppState.updateExpired(this.id, false);
-        this.load();
+        this.load(loadURL);
     };
 
     getBounds = () => {
@@ -452,10 +453,19 @@ export class MattermostBrowserView extends EventEmitter {
             if (!this.browserView || !this.browserView.webContents) {
                 return;
             }
-            const loading = this.browserView.webContents.loadURL(loadURL, {userAgent: composeUserAgent()});
+            const parsedURL = parseURL(loadURL);
-            loading.then(this.loadSuccess(loadURL)).catch(() => {
+            if (!parsedURL) {
-                this.retryLoad = setTimeout(this.retryInBackground(loadURL), RELOAD_INTERVAL);
+                return;
-            });
+            }
+            getServerAPI(
+                parsedURL,
+                false,
+                () => this.reload(loadURL),
+                () => {},
+                (error: Error) => {
+                    this.log.debug(`Cannot reach server: ${error}`);
+                    this.retryLoad = setTimeout(this.retryInBackground(loadURL), RELOAD_INTERVAL);
+                });
         };
     };