Skip to content

Commit

Permalink
Merge pull request #945 from thomasgl-orange/fix-online-check
Browse files Browse the repository at this point in the history
Fix online check with HTTP proxy
  • Loading branch information
jijojosephk authored Sep 11, 2023
2 parents a1910df + febdb6e commit e31f875
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 94 deletions.
1 change: 1 addition & 0 deletions app/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Here is the list of available arguments and its usage:
| disableNotificationSoundIfNotAvailable | Disable chat/meeting start notification sound if status is not Available (e.g. busy, in a call) | true |
| disableNotificationWindowFlash | A flag indicates whether to disable window flashing when there is a notification | false |
| help | show the available commands | false |
| onlineCheckMethod | Type of network test for checking online status, can be: https, dns, native, none | https |
| partition | [BrowserWindow](https://electronjs.org/docs/api/browser-window) webpreferences partition | persist:teams-4-linux |
| proxyServer | Proxy Server with format address:port | None |
| menubar | A value controls the menu bar behaviour (auto/visible/hidden) | auto |
Expand Down
6 changes: 6 additions & 0 deletions app/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ function argv(configPath) {
default: false,
describe: 'Enable debug at start',
type: 'boolean'
},
onlineCheckMethod: {
default: 'https',
describe: 'Type of network test for checking online status.',
type: 'string',
choices: ['https', 'dns', 'native', 'none']
}
})
.parse(process.argv.slice(1));
Expand Down
83 changes: 79 additions & 4 deletions app/connectionManager/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const { ipcMain, powerMonitor } = require('electron');
const { httpHelper } = require('../helpers');
const { ipcMain, net, powerMonitor } = require('electron');
const { LucidLog } = require('lucid-log');

let _ConnectionManager_window = new WeakMap();
Expand Down Expand Up @@ -55,12 +54,12 @@ class ConnectionManager {
async refresh() {
const currentUrl = this.window.webContents.getURL();
const hasUrl = currentUrl && currentUrl.startsWith('https://') ? true : false;
const connected = await httpHelper.isOnline(1000, 1);
const connected = await this.isOnline(1000, 1);
if (!connected) {
this.window.setTitle('Waiting for network...');
this.logger.debug('Waiting for network...');
}
const retryConnected = await httpHelper.isOnline(1000, 30);
const retryConnected = connected || await this.isOnline(1000, 30);
if (retryConnected) {
if (hasUrl) {
this.window.reload();
Expand All @@ -72,6 +71,54 @@ class ConnectionManager {
this.logger.error('No internet connection');
}
}

/**
* @param {number} timeout
* @param {number} retries
* @returns
*/
async isOnline(timeout, retries) {
const onlineCheckMethod = this.config.onlineCheckMethod;
var resolved = false;
for (var i = 1; i <= retries && !resolved; i++) {
resolved = this.isOnlineTest(onlineCheckMethod, this.config.url);
if (!resolved) await sleep(timeout);
}
if (resolved) {
this.logger.debug('Network test successful with method ' + onlineCheckMethod);
} else {
this.logger.debug('Network test failed with method ' + onlineCheckMethod);
}
return resolved;
}

async isOnlineTest(onlineCheckMethod, testUrl) {
switch (onlineCheckMethod) {
case 'none':
// That's more an escape gate in case all methods are broken, it disables
// the network test (assumes we're online).
this.logger.warn('Network test is disabled, assuming online status.');
return true;
case 'dns':
// Sometimes too optimistic, might be false-positive where an HTTP proxy is
// mandatory but not reachable yet.
const testDomain = (new URL(testUrl)).hostname;
this.logger.debug('Testing network using net.resolveHost() for ' + testDomain);
return await isOnlineDns(testDomain);

case 'native':
// Sounds good but be careful, too optimistic in my experience; and at the contrary,
// might also be false negative where no DNS is available for internet domains, but
// an HTTP proxy is actually available and working.
this.logger.debug('Testing network using net.isOnline()');
return net.isOnline();
case 'https':
default:
// Perform an actual HTTPS request, similar to loading the Teams app.
this.logger.debug('Testing network using net.request() for ' + testUrl);
return await isOnlineHttps(testUrl);
}
}
}

/**
Expand Down Expand Up @@ -105,4 +152,32 @@ function assignOnDidFailLoadEventHandler(cm) {
};
}

function sleep(timeout) {
return new Promise(r => setTimeout(r, timeout));
}

function isOnlineHttps(testUrl) {
return new Promise((resolve) => {
var req = net.request({
url: testUrl,
method: 'HEAD'
});
req.on('response', () => {
resolve(true);
});
req.on('error', () => {
resolve(false);
});
req.end();
});
}

function isOnlineDns(testDomain) {
return new Promise((resolve) => {
net.resolveHost(testDomain)
.then(() => resolve(true))
.catch(() => resolve(false));
});
}

module.exports = new ConnectionManager();
36 changes: 0 additions & 36 deletions app/helpers/connectionHelper.js

This file was deleted.

53 changes: 5 additions & 48 deletions app/helpers/httpHelper.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const https = require('https');
const http = require('http');
const { net } = require('electron');

class HTTPHelper {
joinURLs(url1, url2) {
Expand All @@ -11,28 +10,8 @@ class HTTPHelper {
processRequest(url, res, rej);
});
}

/**
* @param {number} timeout
* @param {number} retries
* @param {string} proxyAddress
* @returns
*/
async isOnline(timeout, retries, proxyAddress) {
var resolved = false;
for (var i = 1; i <= retries && !resolved; i++) {
resolved = await isOnlineInternal(proxyAddress);
if (!resolved) await sleep(timeout);
}
return resolved;
}
}

function sleep(timeout) {
return new Promise(r => setTimeout(r, timeout));
}


function removeLeadingSlash(url) {
return (url[0] == '/') ? url = url.substr(1) : url;
}
Expand All @@ -42,7 +21,9 @@ function removeTrailingSlash(url) {
}

function processRequest(url, resolve, reject) {
const request = getHttpClient(url).request(url, (response) => {
const request = net.request(url);

request.on('response', (response) => {
let data = '';
if (response.statusCode >= 200 && response.statusCode < 300) {
response.on('data', (chunk) => {
Expand All @@ -64,28 +45,4 @@ function processRequest(url, resolve, reject) {
request.end();
}

function getHttpClient(url) {
return url.startsWith('http://') ? http : https;
}

async function isOnlineInternal() {
return await isOnlineInternalWithoutProxy();
}

function isOnlineInternalWithoutProxy() {
return new Promise((resolve) => {
var req = https.request({
host: 'teams.microsoft.com',
method: 'CONNECT'
});
req.on('connect', () => {
resolve(true);
});
req.on('error', () => {
resolve(false);
});
req.end();
});
}

module.exports = new HTTPHelper();
module.exports = new HTTPHelper();
6 changes: 2 additions & 4 deletions app/helpers/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
const httpHelper = require('./httpHelper');
const checkConnectivity = require('./connectionHelper');
module.exports = {
httpHelper,
checkConnectivity
};
httpHelper
};
2 changes: 1 addition & 1 deletion app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ if (!gotTheLock) {
ipcMain.handle('play-notification-sound', playNotificationSound);
ipcMain.handle('user-status-changed', userStatusChangedHandler);
ipcMain.handle('set-badge-count', setBadgeCountHandler);
downloadCustomBGServiceRemoteConfig();
}

// eslint-disable-next-line no-unused-vars
Expand Down Expand Up @@ -139,6 +138,7 @@ function onAppTerminated(signal) {
}

function handleAppReady() {
downloadCustomBGServiceRemoteConfig();
process.on('SIGTRAP', onAppTerminated);
process.on('SIGINT', onAppTerminated);
process.on('SIGTERM', onAppTerminated);
Expand Down
15 changes: 15 additions & 0 deletions com.github.IsmaelMartinez.teams_for_linux.appdata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,21 @@
<url type="bugtracker">https://github.com/IsmaelMartinez/teams-for-linux/issues</url>
<launchable type="desktop-id">com.github.IsmaelMartinez.teams_for_linux.desktop</launchable>
<releases>
<release version="1.3.10" date="2023-09-07">
<description>
<ul>
<li>Fix: fixed HTTPS network check from behind an HTTP proxy</li>
<li>New: Added --onlineCheckMethod {https,dns,native,none} to enable picking a different network check method (or disabling)</li>
</ul>
</description>
</release>
<release version="1.3.9" date="2023-09-01">
<description>
<ul>
<li>Update: reordering and cleaning up the list of config options</li>
</ul>
</description>
</release>
<release version="1.3.8" date="2023-08-27">
<description>
<ul>
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "teams-for-linux",
"version": "1.3.9",
"version": "1.3.10",
"main": "app/index.js",
"description": "Unofficial client for Microsoft Teams for Linux",
"homepage": "https://github.com/IsmaelMartinez/teams-for-linux",
Expand Down

0 comments on commit e31f875

Please sign in to comment.