Skip to content

Commit

Permalink
Fix falsely suspended timeago alarms (#5170)
Browse files Browse the repository at this point in the history
* Fix continuous suspension of the timeago alerts

The default hearbeat-setting is 60 seconds, so the delta between two
timeago-checks will always be >15 seconds and the timeago-alarms will
always be suspended (that's what Papertrail also shows).

To fix this, make the delta-check heartbeat-setting-dependant and also simplify
the code by using just 1 variable.

* Add test that verifies the hibernation detection behaviour

The real-world test would be to actually wait for a couple of minutes (with 2 *
heartbeat of default settings) in the unit test, but this is not feasible, so
just modify the heartbeat-setting to a lower value.

I tested it by only running tests inside `tests/timeago.test.js` and by actually
deploying this code and testing the alarms with Pushover and reading the logs in
Papertrail.

Before this change, I saw a 'Hibernation detected' log every minute. After this
change, I didn't see it anymore, probably because the app wasn't actually
hibernated (yet).

* Take sulkaharo's feedback into account and differentiate
between client and server (by introducing it in sandbox.js).

On the client the behaviour is different from the server:

> On client, the issue is browsers stop the execution of JS if the window is not
> visible and the alarm is falsely triggered immediately when the execution is
> resumed, so we need to suspend the alarm for ~10 seconds after the execution has
> resumed to give the client time to update the data to prevent false alarms.

While on the server, the default heartbeat from 60s needs to be taken
into account to prevent the timeago alarm from falsely triggering.
So detect hibernation there if the last check was more than 2 heartbeats ago.

* Fix the tests by adding settings to the context,
which is now required by timeago.

Also, change the timeago test a bit so that it both succeeds
when testing in isolation as when testing it along with the other tests.
  • Loading branch information
fibbers authored and sulkaharo committed Nov 10, 2019
1 parent fcfc209 commit c6e7635
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 6 deletions.
22 changes: 16 additions & 6 deletions lib/plugins/timeago.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var lastSuspendTime = new Date("1900-01-01");

function init(ctx) {
var translate = ctx.language.translate;
var heartbeatMs = ctx.settings.heartbeat * 1000;

var timeago = {
name: 'timeago',
Expand Down Expand Up @@ -64,20 +65,29 @@ function init(ctx) {
};

timeago.checkStatus = function checkStatus(sbx) {

// Check if the app has been suspended; if yes, snooze data missing alarmn for 15 seconds
var now = new Date();
var delta = now.getTime() - lastChecked.getTime();
lastChecked = now;

if (delta > 15 * 1000) { // Looks like we've been hibernating
lastSuspendTime = now;
}
function isHibernationDetected() {
if (sbx.runtimeEnvironment === 'client') {
if (delta > 15 * 1000) { // Looks like we've been hibernating
lastSuspendTime = now;
}

var timeSinceLastSuspended = now.getTime() - lastSuspendTime.getTime();
var timeSinceLastSuspended = now.getTime() - lastSuspendTime.getTime();

if (timeSinceLastSuspended < (10 * 1000)) {
return timeSinceLastSuspended < (10 * 1000);
} else if (sbx.runtimeEnvironment === 'server') {
return delta > 2 * heartbeatMs;
} else {
console.error('Cannot detect hibernation, because runtimeEnvironment is not detected from sbx.runtimeEnvironment:', sbx.runtimeEnvironment);
return false;
}
}

if (isHibernationDetected()) {
console.log('Hibernation detected, suspending timeago alarm');
return 'current';
}
Expand Down
2 changes: 2 additions & 0 deletions lib/sandbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ function init () {
sbx.serverInit = function serverInit (env, ctx) {
reset();

sbx.runtimeEnvironment = 'server';
sbx.time = Date.now();
sbx.settings = env.settings;
sbx.data = ctx.ddata.clone();
Expand Down Expand Up @@ -83,6 +84,7 @@ function init () {
sbx.clientInit = function clientInit (ctx, time, data) {
reset();

sbx.runtimeEnvironment = 'client';
sbx.settings = ctx.settings;
sbx.showPlugins = ctx.settings.showPlugins;
sbx.time = time;
Expand Down
1 change: 1 addition & 0 deletions tests/bgnow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var SIX_MINS = 360000;
describe('BG Now', function ( ) {
var ctx = {
language: require('../lib/language')()
, settings: require('../lib/settings')()
};

ctx.levels = require('../lib/levels');
Expand Down
1 change: 1 addition & 0 deletions tests/iob.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ describe('IOB', function() {
var ctx = {};
ctx.language = require('../lib/language')();
ctx.language.set('en');
ctx.settings = require('../lib/settings')();

var iob = require('../lib/plugins/iob')(ctx);

Expand Down
1 change: 1 addition & 0 deletions tests/loop.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var moment = require('moment');

var ctx = {
language: require('../lib/language')()
, settings: require('../lib/settings')()
};
ctx.language.set('en');
var env = require('../env')();
Expand Down
1 change: 1 addition & 0 deletions tests/openaps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var moment = require('moment');

var ctx = {
language: require('../lib/language')()
, settings: require('../lib/settings')()
};
ctx.language.set('en');
var env = require('../env')();
Expand Down
1 change: 1 addition & 0 deletions tests/pump.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var moment = require('moment');

var ctx = {
language: require('../lib/language')()
, settings: require('../lib/settings')()
};
ctx.language.set('en');
var env = require('../env')();
Expand Down
30 changes: 30 additions & 0 deletions tests/timeago.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ describe('timeago', function() {
ctx.ddata = require('../lib/data/ddata')();
ctx.notifications = require('../lib/notifications')(env, ctx);
ctx.language = require('../lib/language')();
ctx.settings = require('../lib/settings')();
ctx.settings.heartbeat = 0.5; // short heartbeat to speedup tests

var timeago = require('../lib/plugins/timeago')(ctx);

Expand Down Expand Up @@ -41,6 +43,34 @@ describe('timeago', function() {
done();
});

it('should suspend alarms due to hibernation when 2 heartbeats are skipped on server', function() {
ctx.ddata.sgvs = [{ mills: Date.now() - times.mins(16).msecs, mgdl: 100, type: 'sgv' }];

var sbx = freshSBX()
var status = timeago.checkStatus(sbx);
// By default (no hibernation detected) a warning should be given
// we force no hibernation by checking status twice
status = timeago.checkStatus(sbx);
should.equal(status, 'warn');

// 10ms more than suspend-threshold to prevent flapping tests
var timeoutMs = 2 * ctx.settings.heartbeat * 1000 + 100;
return new Promise(function(resolve, reject) {
setTimeout(function() {
status = timeago.checkStatus(sbx);
// Because hibernation should now be detected, no warning should be given
should.equal(status, 'current');

// We immediately ask status again, so hibernation should not be detected anymore,
// and we should receive a warning again
status = timeago.checkStatus(sbx);
should.equal(status, 'warn');

resolve()
}, timeoutMs)
})
});

it('should trigger a warning when data older than 15m', function(done) {
ctx.notifications.initRequests();
ctx.ddata.sgvs = [{ mills: Date.now() - times.mins(16).msecs, mgdl: 100, type: 'sgv' }];
Expand Down

0 comments on commit c6e7635

Please sign in to comment.