Skip to content

Commit 46a27f7

Browse files
fix: recover API when startup notification is missed
Previously the remote API could get stuck as "Not initialized" if the browser's startup signal was missed, breaking every request until a manual refresh. It now fetches the current state on demand and answers the request as soon as the mirror responds. Ref Jopyth#412
1 parent b81f5a3 commit 46a27f7

12 files changed

Lines changed: 141 additions & 66 deletions

API/api.js

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,7 @@ module.exports = {
149149
// Route for testing the api at http://mirror:8080/api/test
150150
this.expressRouter.route(["/test", "/"]). // Test without apiKey
151151
get((request, response) => {
152-
if (!this.checkInitialized(response)) { return; }
153-
response.json({success: true});
152+
response.json({success: true, initialized: Boolean(this.initialized)});
154153
});
155154

156155
/*
@@ -400,7 +399,10 @@ module.exports = {
400399
},
401400

402401
answerModuleApi (request, response) {
403-
if (!this.checkInitialized(response)) { return; }
402+
this.requireLiveState(response, () => this.resolveModuleApi(request, response));
403+
},
404+
405+
resolveModuleApi (request, response) {
404406
const dataMerged = this.mergeData().data;
405407

406408
if (!request.params.moduleName) {
@@ -568,14 +570,6 @@ module.exports = {
568570

569571
},
570572

571-
checkInitialized (response) {
572-
if (!this.initialized) {
573-
this.sendResponse(response, "Not initialized, have you opened or refreshed your browser since the last time you started MagicMirror²?");
574-
return false;
575-
}
576-
return true;
577-
},
578-
579573
updateModuleApiMenu () {
580574
if (!this.thisConfig.showModuleApiMenu) { return; }
581575

node_helper.js

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -508,43 +508,37 @@ module.exports = NodeHelper.create({
508508
},
509509

510510
handleGetModules (query, response) {
511-
if (!this.checkInitialized(response)) { return; }
512-
this.callAfterUpdate(() => {
511+
this.requireLiveState(response, () => {
513512
this.sendResponse(response, undefined, {query, data: this.configData.moduleData});
514513
});
515514
},
516515

517516
handleGetBrightness (query, response) {
518-
if (!this.checkInitialized(response)) { return; }
519-
this.callAfterUpdate(() => {
517+
this.requireLiveState(response, () => {
520518
this.sendResponse(response, undefined, {query, result: this.configData.brightness});
521519
});
522520
},
523521

524522
handleGetTemp (query, response) {
525-
if (!this.checkInitialized(response)) { return; }
526-
this.callAfterUpdate(() => {
523+
this.requireLiveState(response, () => {
527524
this.sendResponse(response, undefined, {query, result: this.configData.temp});
528525
});
529526
},
530527

531528
handleGetZoom (query, response) {
532-
if (!this.checkInitialized(response)) { return; }
533-
this.callAfterUpdate(() => {
529+
this.requireLiveState(response, () => {
534530
this.sendResponse(response, undefined, {query, result: this.configData.zoom});
535531
});
536532
},
537533

538534
handleGetBackgroundColor (query, response) {
539-
if (!this.checkInitialized(response)) { return; }
540-
this.callAfterUpdate(() => {
535+
this.requireLiveState(response, () => {
541536
this.sendResponse(response, undefined, {query, result: this.configData.backgroundColor});
542537
});
543538
},
544539

545540
handleGetFontColor (query, response) {
546-
if (!this.checkInitialized(response)) { return; }
547-
this.callAfterUpdate(() => {
541+
this.requireLiveState(response, () => {
548542
this.sendResponse(response, undefined, {query, result: this.configData.fontColor});
549543
});
550544
},
@@ -604,14 +598,36 @@ module.exports = NodeHelper.create({
604598

605599
callAfterUpdate (callback, timeout = 3000) {
606600
let isDone = false;
607-
const once = () => {
601+
const once = (didUpdate) => {
608602
if (isDone) { return; }
609603
isDone = true;
610-
callback();
604+
callback(didUpdate);
611605
};
612-
this.waiting.push({run: once});
606+
this.waiting.push({run: () => once(true)});
613607
this.sendSocketNotification("UPDATE");
614-
setTimeout(once, timeout);
608+
setTimeout(() => once(false), timeout);
609+
},
610+
611+
/**
612+
* Serve a request that depends on live frontend state.
613+
*
614+
* Instead of relying on a boot-time notification having been received, this
615+
* actively pulls a fresh CURRENT_STATUS from the frontend (via callAfterUpdate).
616+
* It is self-healing: even if the frontend missed DOM_OBJECTS_CREATED at
617+
* startup, the triggered UPDATE makes it resend its state. An error is only
618+
* returned when no state can be obtained at all (no browser connected).
619+
* @param {object} response - Express or socket response object
620+
* @param {() => void} callback - Invoked once live state is available
621+
* @returns {void}
622+
*/
623+
requireLiveState (response, callback) {
624+
this.callAfterUpdate((didUpdate) => {
625+
if (didUpdate || this.initialized) {
626+
callback();
627+
} else {
628+
this.sendResponse(response, "Not connected to the MagicMirror² frontend. Open or refresh a browser pointing at the mirror.");
629+
}
630+
});
615631
},
616632

617633
delayedQuery (query, response) {
@@ -1058,13 +1074,14 @@ module.exports = NodeHelper.create({
10581074
case "CURRENT_STATUS":
10591075
this.configData = payload;
10601076
this.thisConfig = payload.remoteConfig;
1061-
if (this.initialized) {
1062-
for (const o of this.waiting) { o.run(); }
1063-
this.waiting = [];
1064-
} else {
1065-
// Do anything else required to initialize
1066-
this.initialized = true;
1067-
}
1077+
this.initialized = true;
1078+
1079+
/*
1080+
* Always drain requests waiting for fresh frontend state. The pull is
1081+
* idempotent, so a missed boot-time notification is no longer fatal.
1082+
*/
1083+
for (const o of this.waiting) { o.run(); }
1084+
this.waiting = [];
10681085

10691086
break;
10701087
case "REQUEST_DEFAULT_SETTINGS":

tests/integration/api.multi-instance.test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ function createMockContext (app, notifications) {
9393
handleGetModuleInstalled: helperModule.handleGetModuleInstalled,
9494
getDataHandlers: helperModule.getDataHandlers,
9595
sendResponse: helperModule.sendResponse,
96-
checkInitialized: helperModule.checkInitialized,
96+
requireLiveState: (_res, cb) => cb(),
97+
resolveModuleApi: apiModule.resolveModuleApi,
9798
getActionHandlers: helperModule.getActionHandlers,
9899
executeQuery: helperModule.executeQuery,
99100
handleSimpleSocketNotification: helperModule.handleSimpleSocketNotification,
@@ -129,7 +130,6 @@ function createMockContext (app, notifications) {
129130
mockContext.handleGetModuleInstalled = mockContext.handleGetModuleInstalled.bind(mockContext);
130131
mockContext.getDataHandlers = mockContext.getDataHandlers.bind(mockContext);
131132
mockContext.sendResponse = mockContext.sendResponse.bind(mockContext);
132-
mockContext.checkInitialized = mockContext.checkInitialized.bind(mockContext);
133133
mockContext.getActionHandlers = mockContext.getActionHandlers.bind(mockContext);
134134
mockContext.executeQuery = mockContext.executeQuery.bind(mockContext);
135135
mockContext.handleSimpleSocketNotification = mockContext.handleSimpleSocketNotification.bind(mockContext);
@@ -138,6 +138,7 @@ function createMockContext (app, notifications) {
138138
mockContext.checkDelay = mockContext.checkDelay.bind(mockContext);
139139
mockContext.answerNotifyApi = mockContext.answerNotifyApi.bind(mockContext);
140140
mockContext.answerModuleApi = mockContext.answerModuleApi.bind(mockContext);
141+
mockContext.resolveModuleApi = mockContext.resolveModuleApi.bind(mockContext);
141142
mockContext.mergeData = mockContext.mergeData.bind(mockContext);
142143

143144
return mockContext;

tests/integration/api.smoke.test.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,12 @@ describe("API HTTP-Layer Smoke Tests", () => {
121121
return !error;
122122
},
123123

124-
checkInitialized (res) {
125-
if (!mockContext.initialized) {
126-
mockContext.sendResponse(res, new Error("System not initialized"));
127-
return false;
124+
requireLiveState (res, cb) {
125+
if (mockContext.initialized) {
126+
cb();
127+
} else {
128+
mockContext.sendResponse(res, new Error("Not connected to frontend"));
128129
}
129-
return true;
130130
},
131131

132132
getApiKey () {
@@ -159,7 +159,8 @@ describe("API HTTP-Layer Smoke Tests", () => {
159159

160160
// Bind API methods
161161
answerNotifyApi: apiModule.answerNotifyApi,
162-
answerModuleApi: apiModule.answerModuleApi
162+
answerModuleApi: apiModule.answerModuleApi,
163+
resolveModuleApi: apiModule.resolveModuleApi
163164
};
164165

165166
// Bind all methods to context
@@ -169,6 +170,7 @@ describe("API HTTP-Layer Smoke Tests", () => {
169170
mockContext.getDataHandlers = mockContext.getDataHandlers.bind(mockContext);
170171
mockContext.answerNotifyApi = mockContext.answerNotifyApi.bind(mockContext);
171172
mockContext.answerModuleApi = mockContext.answerModuleApi.bind(mockContext);
173+
mockContext.resolveModuleApi = mockContext.resolveModuleApi.bind(mockContext);
172174

173175
// Bind API methods to context
174176
apiModule.createApiRoutes.call(mockContext);

tests/unit/answerGet.contract.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ function freshHelper (overrides = {}) {
2222
helper.configOnHd = {language: "en", modules: []};
2323
helper.configData = {moduleData: []};
2424
helper.sendSocketNotification = () => {};
25-
helper.checkInitialized = () => true;
25+
helper.initialized = true;
2626
helper.callAfterUpdate = (function_) => function_();
2727
helper.removeDefaultValues = (config) => config;
2828
helper.answerGet = helperFactory.answerGet.bind(helper);

tests/unit/api.answerModuleApi.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ function makeContext (overrides = {}) {
1313
mergeData: () => ({success: true, data: context.configData.moduleData}),
1414
sendSocketNotification: () => {},
1515
sendResponse: () => {},
16-
checkInitialized: () => true,
16+
requireLiveState: (_res, cb) => cb(),
17+
resolveModuleApi: apiModule.resolveModuleApi,
1718
translate: (s) => s,
1819
thisConfig: {},
1920
...overrides

tests/unit/api.delayedFlow.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ function makeContext (overrides = {}) {
1111
translation: {},
1212
sendSocketNotification: () => {},
1313
sendResponse: () => {},
14-
checkInitialized: () => true,
1514
checkDelay: apiModule.checkDelay,
1615
translate: (s) => s,
1716
delayedQuery: () => {},

tests/unit/api.helpers.test.js

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ function makeContext (overrides = {}) {
1212
translation: {},
1313
sendSocketNotification: () => {},
1414
sendResponse: () => {},
15-
checkInitialized: () => true,
1615
translate: (s) => s,
1716
thisConfig: {},
1817
...overrides
@@ -152,22 +151,4 @@ describe("API helpers", () => {
152151
assert.deepEqual(captured.payload.payload, {param: "raw-value", foo: "bar"});
153152
});
154153
});
155-
156-
describe("checkInitialized", () => {
157-
test("returns false and sends response when helper is not initialized", () => {
158-
let capturedError;
159-
const context = makeContext({
160-
initialized: false,
161-
sendResponse: (_res, error) => {
162-
capturedError = error;
163-
}
164-
});
165-
166-
const result = apiModule.checkInitialized.call(context, {status: () => ({json: () => {}})});
167-
168-
assert.equal(result, false);
169-
assert.equal(typeof capturedError, "string");
170-
assert.ok(capturedError.includes("Not initialized"));
171-
});
172-
});
173154
});

tests/unit/executeQuery.error.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ function freshHelper (overrides = {}) {
2424
helper.thisConfig = {classes: {}};
2525
helper.configOnHd = {};
2626
helper.configData = {moduleData: []};
27-
helper.checkInitialized = () => true;
2827
helper.executeQuery = helperFactory.executeQuery.bind(helper);
2928
helper.handleNotification = helperFactory.handleNotification.bind(helper);
3029
helper.handleSimpleSocketNotification = helperFactory.handleSimpleSocketNotification.bind(helper);

tests/unit/executeQuery.handlers.test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,47 @@ describe("callAfterUpdate", () => {
8080
});
8181
});
8282

83+
describe("requireLiveState", () => {
84+
test("serves fresh state when frontend responds to the pull", () => {
85+
const helper = freshHelper();
86+
helper.initialized = false;
87+
helper.callAfterUpdate = (cb) => cb(true);
88+
helper.requireLiveState = helperFactory.requireLiveState.bind(helper);
89+
90+
let isServed = false;
91+
helper.requireLiveState({}, () => { isServed = true; });
92+
93+
assert.ok(isServed);
94+
assert.equal(helper.__responses.length, 0);
95+
});
96+
97+
test("serves existing state when the pull times out but state is present", () => {
98+
const helper = freshHelper();
99+
helper.initialized = true;
100+
helper.callAfterUpdate = (cb) => cb(false);
101+
helper.requireLiveState = helperFactory.requireLiveState.bind(helper);
102+
103+
let isServed = false;
104+
helper.requireLiveState({}, () => { isServed = true; });
105+
106+
assert.ok(isServed);
107+
});
108+
109+
test("responds with an error when no state can be obtained at all", () => {
110+
const helper = freshHelper();
111+
helper.initialized = false;
112+
helper.callAfterUpdate = (cb) => cb(false);
113+
helper.requireLiveState = helperFactory.requireLiveState.bind(helper);
114+
115+
let isServed = false;
116+
helper.requireLiveState({}, () => { isServed = true; });
117+
118+
assert.equal(isServed, false);
119+
assert.equal(helper.__responses.length, 1);
120+
assert.ok(String(helper.__responses[0].err).includes("Not connected"));
121+
});
122+
});
123+
83124
describe("getActionHandlers", () => {
84125
test("returns object with all action handlers", () => {
85126
const helper = freshHelper();

0 commit comments

Comments
 (0)