From 0b40aca7f545b0547ff8bb1c093e18c03518bb29 Mon Sep 17 00:00:00 2001 From: Brad Miller Date: Mon, 4 Jan 2021 15:37:06 -0800 Subject: [PATCH 1/5] Cleanup async details * Make sure all activecodes call manage_scrubber correctly * document checkServer --- runestone/activecode/js/activecode.js | 2 +- runestone/activecode/js/activecode_html.js | 11 ++--------- runestone/activecode/js/activecode_js.js | 7 ++----- runestone/activecode/js/activecode_sql.js | 9 +-------- runestone/activecode/js/livecode.js | 12 +++--------- runestone/common/js/runestonebase.js | 7 ++++++- 6 files changed, 15 insertions(+), 33 deletions(-) diff --git a/runestone/activecode/js/activecode.js b/runestone/activecode/js/activecode.js index ccb954586..113372176 100755 --- a/runestone/activecode/js/activecode.js +++ b/runestone/activecode/js/activecode.js @@ -1164,7 +1164,7 @@ Yet another is that there is an internal error. The internal error message is: if (this.historyScrubber == null) { saveCode = "False"; } - return Promise.resolve(saveCode); + return saveCode; } async checkCurrentAnswer() { diff --git a/runestone/activecode/js/activecode_html.js b/runestone/activecode/js/activecode_html.js index c50839afa..49c4f130f 100644 --- a/runestone/activecode/js/activecode_html.js +++ b/runestone/activecode/js/activecode_html.js @@ -11,15 +11,8 @@ export default class HTMLActiveCode extends ActiveCode { async runProg() { var prog = await this.buildProg(true); - var scrubber_dfd, history_dfd, saveCode; - saveCode = "True"; - var __ret = await this.manage_scrubber( - scrubber_dfd, - history_dfd, - saveCode - ); - history_dfd = __ret.history_dfd; - saveCode = __ret.saveCode; + let saveCode = "True"; + saveCode = await this.manage_scrubber(saveCode); $(this.output).text(""); if (!this.alignVertical) { $(this.codeDiv).switchClass("col-md-12", "col-md-6", { diff --git a/runestone/activecode/js/activecode_js.js b/runestone/activecode/js/activecode_js.js index afae894da..4d9776b77 100644 --- a/runestone/activecode/js/activecode_js.js +++ b/runestone/activecode/js/activecode_js.js @@ -29,11 +29,10 @@ export default class JSActiveCode extends ActiveCode { } return str; } - runProg() { + async runProg() { var _this = this; var prog = this.buildProg(true); var einfo; - var scrubber_dfd, history_dfd; var saveCode = "True"; var write = function (str) { _this.output.innerHTML += _this.outputfun(str); @@ -42,9 +41,7 @@ export default class JSActiveCode extends ActiveCode { if (!str) str = ""; _this.output.innerHTML += _this.outputfun(str) + "
"; }; - var __ret = this.manage_scrubber(scrubber_dfd, history_dfd, saveCode); - history_dfd = __ret.history_dfd; - saveCode = __ret.saveCode; + saveCode = await this.manage_scrubber(saveCode); $(this.eContainer).remove(); $(this.output).text(""); $(this.outDiv).show({ duration: 700, queue: false }); diff --git a/runestone/activecode/js/activecode_sql.js b/runestone/activecode/js/activecode_sql.js index d66312aec..ce9b6ca7d 100644 --- a/runestone/activecode/js/activecode_sql.js +++ b/runestone/activecode/js/activecode_sql.js @@ -79,7 +79,6 @@ export default class SQLActiveCode extends ActiveCode { if (typeof noUI !== "boolean") { noUI = false; } - var scrubber_dfd, history_dfd; // Clear any old results this.saveCode = "True"; let divid = this.divid + "_sql_out"; @@ -153,13 +152,7 @@ export default class SQLActiveCode extends ActiveCode { } try { - var __ret = await this.manage_scrubber( - scrubber_dfd, - history_dfd, - this.saveCode - ); - history_dfd = __ret.history_dfd; - this.saveCode = __ret.saveCode; + this.saveCode = await this.manage_scrubber(this.saveCode); if (this.slideit) { $(this.historyScrubber).on( "slidechange", diff --git a/runestone/activecode/js/livecode.js b/runestone/activecode/js/livecode.js index 3db6bf526..ca12e02ab 100644 --- a/runestone/activecode/js/livecode.js +++ b/runestone/activecode/js/livecode.js @@ -57,8 +57,9 @@ export default class LiveCode extends ActiveCode { } catch (e) { this.addJobeErrorMessage($.i18n("msg_activecode_server_comm_err")); $(this.runButton).removeAttr("disabled"); + return `fail: ${e}`; } - return Promise.resolve("done"); + return "success"; } /** * Note: @@ -67,7 +68,6 @@ export default class LiveCode extends ActiveCode { */ async runSetup() { var stdin; - var scrubber_dfd, history_dfd; var source; var saveCode = "True"; var sfilemap = { @@ -108,13 +108,7 @@ export default class LiveCode extends ActiveCode { return; } - var __ret = await this.manage_scrubber( - scrubber_dfd, - history_dfd, - saveCode - ); - history_dfd = __ret.history_dfd; - saveCode = __ret.saveCode; + saveCode = await this.manage_scrubber(saveCode); // assemble parameters for JOBE var paramlist = [ diff --git a/runestone/common/js/runestonebase.js b/runestone/common/js/runestonebase.js index c17b2e25c..45815ef02 100644 --- a/runestone/common/js/runestonebase.js +++ b/runestone/common/js/runestonebase.js @@ -140,7 +140,12 @@ export default class RunestoneBase { } return post_promise; } - /* Checking/loading from storage */ + /* Checking/loading from storage + **WARNING:** DO NOT `await` this function! + This function, although async, does not explicitly resolve its promise by returning a value. The reason for this is because it is called by the constructor for nearly every component. In Javascript constructors cannot be async! + + One of the recommended ways to handle the async requirements from within a constructor is to use an attribute as a promise and resolve that attribute at the appropriate time. + */ async checkServer(eventInfo) { // Check if the server has stored answer let self = this; From 28708517607bfa226aa9a75770ff7368f3b4ee0d Mon Sep 17 00:00:00 2001 From: Brad Miller Date: Tue, 5 Jan 2021 16:02:57 -0800 Subject: [PATCH 2/5] Better handling of server errors --- runestone/activecode/js/livecode.js | 8 ++++++++ runestone/timed/js/timed.js | 20 ++++++++++---------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/runestone/activecode/js/livecode.js b/runestone/activecode/js/livecode.js index ca12e02ab..9975ea163 100644 --- a/runestone/activecode/js/livecode.js +++ b/runestone/activecode/js/livecode.js @@ -51,6 +51,14 @@ export default class LiveCode extends ActiveCode { await this.runSetup(); try { let res = await this.submitToJobe(); + if (!res.ok) { + this.addJobeErrorMessage( + $.i18n(`Server Error: ${res.statusText}`) + ); + this.run_promise = Promise.resolve(); + $(this.runButton).removeAttr("disabled"); + return "fail"; + } this.run_promise = Promise.resolve(); let runResults = await res.json(); this.processJobeResponse(runResults); diff --git a/runestone/timed/js/timed.js b/runestone/timed/js/timed.js index c5d92c3a9..bb759d56f 100644 --- a/runestone/timed/js/timed.js +++ b/runestone/timed/js/timed.js @@ -136,7 +136,7 @@ export default class Timed extends RunestoneBase { /*=============================== === Generating new Timed HTML === ===============================*/ - renderTimedAssess() { + async renderTimedAssess() { console.log("rendering timed "); // create renderedQuestionArray returns a promise // @@ -146,7 +146,7 @@ export default class Timed extends RunestoneBase { } this.renderContainer(); this.renderTimer(); - this.renderControlButtons(); + await this.renderControlButtons(); this.assessDiv.appendChild(this.timedDiv); // This can't be appended in renderContainer because then it renders above the timer and control buttons. if (this.renderedQuestionArray.length > 1) this.renderNavControls(); this.renderSubmitButton(); @@ -211,13 +211,13 @@ export default class Timed extends RunestoneBase { this.startBtn.textContent = "Start"; this.startBtn.addEventListener( "click", - function () { + async function () { $(this.finishButton).hide(); // hide the finish button for now let mess = document.createElement("p"); mess.innerHTML = "Warning: You will lose all of your work if you close this tab or the browser. Make sure you click the Finish Exam button to submit your work!"; this.controlDiv.appendChild(mess); - this.renderTimedQuestion(); + await this.renderTimedQuestion(); this.startAssessment(); }.bind(this), false @@ -301,7 +301,7 @@ export default class Timed extends RunestoneBase { // Next and Prev Listener this.pagNavList.addEventListener( "click", - function (event) { + async function (event) { if ( this.renderedQuestionArray[this.currentQuestionIndex] .state == "broken_exam" @@ -343,7 +343,7 @@ export default class Timed extends RunestoneBase { } this.currentQuestionIndex--; } - this.renderTimedQuestion(); + await this.renderTimedQuestion(); this.ensureButtonSafety(); for (var i = 0; i < this.qNumList.childNodes.length; i++) { for ( @@ -367,7 +367,7 @@ export default class Timed extends RunestoneBase { // Numbered Listener this.qNumList.addEventListener( "click", - function (event) { + async function (event) { if ( this.renderedQuestionArray[this.currentQuestionIndex] .state == "broken_exam" @@ -423,7 +423,7 @@ export default class Timed extends RunestoneBase { this.currentQuestionIndex + ")" ).addClass("active"); - this.renderTimedQuestion(); + await this.renderTimedQuestion(); this.ensureButtonSafety(); }.bind(this), false @@ -1012,7 +1012,7 @@ export default class Timed extends RunestoneBase { this.taken = 0; } } - restoreAnswers(data) { + async restoreAnswers(data) { this.taken = 1; var tmpArr; if (data === "") { @@ -1072,7 +1072,7 @@ export default class Timed extends RunestoneBase { } this.handlePrevAssessment(); } - this.renderTimedQuestion(); + await this.renderTimedQuestion(); this.displayScore(); this.showTime(); } From 7b5962fc68630b829006ae22acb96b68f594b257 Mon Sep 17 00:00:00 2001 From: Brad Miller Date: Tue, 5 Jan 2021 17:31:04 -0800 Subject: [PATCH 3/5] set this.saveCode properly --- runestone/activecode/js/livecode.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runestone/activecode/js/livecode.js b/runestone/activecode/js/livecode.js index 9975ea163..3c9548d98 100644 --- a/runestone/activecode/js/livecode.js +++ b/runestone/activecode/js/livecode.js @@ -116,7 +116,7 @@ export default class LiveCode extends ActiveCode { return; } - saveCode = await this.manage_scrubber(saveCode); + this.saveCode = await this.manage_scrubber(saveCode); // assemble parameters for JOBE var paramlist = [ @@ -296,7 +296,6 @@ export default class LiveCode extends ActiveCode { } else { logresult = result.outcome; } - this.saveCode = "True"; this.errinfo = logresult; switch (result.outcome) { case 15: { From 88fea9aa89e1f58c0060ded5ae5d80aa3a643414 Mon Sep 17 00:00:00 2001 From: Brad Miller Date: Sat, 9 Jan 2021 15:44:51 -0800 Subject: [PATCH 4/5] No need for run_promise anymore --- runestone/activecode/js/activecode.js | 105 ++++++++------------ runestone/activecode/js/livecode.js | 2 - runestone/activecode/js/timed_activecode.js | 4 +- 3 files changed, 46 insertions(+), 65 deletions(-) diff --git a/runestone/activecode/js/activecode.js b/runestone/activecode/js/activecode.js index 113372176..e6c8c4bb6 100755 --- a/runestone/activecode/js/activecode.js +++ b/runestone/activecode/js/activecode.js @@ -1176,27 +1176,24 @@ Yet another is that there is an internal error. The internal error message is: } logCurrentAnswer() { - let self = this; - this.run_promise.then(function () { - self.logRunEvent({ - div_id: self.divid, - code: self.editor.getValue(), - lang: self.language, - errinfo: self.errinfo, - to_save: self.saveCode, - prefix: self.pretext, - suffix: self.suffix, - partner: self.partner, - }); // Log the run event - // If unit tests were run there will be a unit_results - if (self.unit_results) { - self.logBookEvent({ - act: self.unit_results, - div_id: self.divid, - event: "unittest", - }); - } - }); + this.logRunEvent({ + div_id: this.divid, + code: this.editor.getValue(), + lang: this.language, + errinfo: this.errinfo, + to_save: this.saveCode, + prefix: this.pretext, + suffix: this.suffix, + partner: this.partner, + }); // Log the run event + // If unit tests were run there will be a unit_results + if (this.unit_results) { + this.logBookEvent({ + act: this.unit_results, + div_id: this.divid, + event: "unittest", + }); + } } renderFeedback() { @@ -1274,7 +1271,6 @@ Yet another is that there is an internal error. The internal error message is: this.setTimeLimit(); (Sk.TurtleGraphics || (Sk.TurtleGraphics = {})).target = this.graphics; Sk.canvas = this.graphics.id; //todo: get rid of this here and in image - let promise_list = []; if (!noUI) { $(this.runButton).attr("disabled", "disabled"); $(this.historyScrubber).off("slidechange"); @@ -1285,47 +1281,34 @@ Yet another is that there is an internal error. The internal error message is: }); this.saveCode = await this.manage_scrubber(this.saveCode); } - this.run_promise = Sk.misceval.asyncToPromise(function () { - return Sk.importMainWithBody("", false, prog, true); - }); - promise_list.push(this.run_promise); - // Make sure that the history scrubber is fully initialized AND the code has been run - // before we start logging stuff. - var self = this; - Promise.all(promise_list) - .then( - function () { - $(this.runButton).removeAttr("disabled"); - if (!noUI) { - if (this.slideit) { - $(this.historyScrubber).on( - "slidechange", - this.slideit.bind(this) - ); - } - $(this.historyScrubber).slider("enable"); - } - this.errLastRun = false; - this.errinfo = "success"; - }.bind(this) - ) - .catch(function (err) { - $(self.runButton).removeAttr("disabled"); - $(self.historyScrubber).on( - "slidechange", - self.slideit.bind(self) - ); - $(self.historyScrubber).slider("enable"); - self.errinfo = err.toString(); - self.addErrorMessage(err); - }); - if (typeof window.allVisualizers != "undefined") { - $.each(window.allVisualizers, function (i, e) { - e.redrawConnectors(); + try { + await Sk.misceval.asyncToPromise(function () { + return Sk.importMainWithBody("", false, prog, true); }); + if (!noUI) { + if (this.slideit) { + $(this.historyScrubber).on( + "slidechange", + this.slideit.bind(this) + ); + } + $(this.historyScrubber).slider("enable"); + } + this.errLastRun = false; + this.errinfo = "success"; + } catch (err) { + $(this.historyScrubber).on("slidechange", this.slideit.bind(this)); + $(this.historyScrubber).slider("enable"); + this.errinfo = err.toString(); + this.addErrorMessage(err); + } finally { + $(this.runButton).removeAttr("disabled"); + if (typeof window.allVisualizers != "undefined") { + $.each(window.allVisualizers, function (i, e) { + e.redrawConnectors(); + }); + } } - - return this.run_promise; } disableInteraction() { diff --git a/runestone/activecode/js/livecode.js b/runestone/activecode/js/livecode.js index 3c9548d98..3aba3154f 100644 --- a/runestone/activecode/js/livecode.js +++ b/runestone/activecode/js/livecode.js @@ -55,11 +55,9 @@ export default class LiveCode extends ActiveCode { this.addJobeErrorMessage( $.i18n(`Server Error: ${res.statusText}`) ); - this.run_promise = Promise.resolve(); $(this.runButton).removeAttr("disabled"); return "fail"; } - this.run_promise = Promise.resolve(); let runResults = await res.json(); this.processJobeResponse(runResults); } catch (e) { diff --git a/runestone/activecode/js/timed_activecode.js b/runestone/activecode/js/timed_activecode.js index 47c86e28d..4d22a9eb5 100644 --- a/runestone/activecode/js/timed_activecode.js +++ b/runestone/activecode/js/timed_activecode.js @@ -99,8 +99,8 @@ export class TimedActiveCode extends ActiveCode { // for timed exams we need to call runProg and tell it that there is // no GUI for sliders or other things. We also do not want it to log // the answers. - checkCurrentAnswer() { - this.run_promise = this.runProg(true, false); + async checkCurrentAnswer() { + await this.runProg(true, false); } } From 7d8db112f8d46aec86e87a09b198b97474967a9e Mon Sep 17 00:00:00 2001 From: Brad Miller Date: Sun, 10 Jan 2021 07:47:54 -0800 Subject: [PATCH 5/5] Further cleanup using async/await --- .eslintrc.js | 1 + runestone/activecode/js/activecode_html.js | 13 ++----------- runestone/activecode/js/activecode_js.js | 20 ++++---------------- 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index be6c275b8..471c53e9e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -12,5 +12,6 @@ module.exports = { rules: {}, globals: { eBookConfig: "readonly", + require: "readonly", }, }; diff --git a/runestone/activecode/js/activecode_html.js b/runestone/activecode/js/activecode_html.js index 49c4f130f..5912bd588 100644 --- a/runestone/activecode/js/activecode_html.js +++ b/runestone/activecode/js/activecode_html.js @@ -12,7 +12,7 @@ export default class HTMLActiveCode extends ActiveCode { async runProg() { var prog = await this.buildProg(true); let saveCode = "True"; - saveCode = await this.manage_scrubber(saveCode); + this.saveCode = await this.manage_scrubber(saveCode); $(this.output).text(""); if (!this.alignVertical) { $(this.codeDiv).switchClass("col-md-12", "col-md-6", { @@ -25,17 +25,8 @@ export default class HTMLActiveCode extends ActiveCode { "" + prog; this.output.srcdoc = prog; - this.logRunEvent({ - div_id: this.divid, - code: this.editor.getValue(), - errinfo: "success", - to_save: saveCode, - prefix: this.pretext, - suffix: this.suffix, - lang: this.language, - partner: this.partner, - }); // Log the run event } + createOutput() { this.alignVertical = true; var outDiv = document.createElement("div"); diff --git a/runestone/activecode/js/activecode_js.js b/runestone/activecode/js/activecode_js.js index 4d9776b77..0217adaec 100644 --- a/runestone/activecode/js/activecode_js.js +++ b/runestone/activecode/js/activecode_js.js @@ -31,8 +31,7 @@ export default class JSActiveCode extends ActiveCode { } async runProg() { var _this = this; - var prog = this.buildProg(true); - var einfo; + var prog = await this.buildProg(true); var saveCode = "True"; var write = function (str) { _this.output.innerHTML += _this.outputfun(str); @@ -41,27 +40,16 @@ export default class JSActiveCode extends ActiveCode { if (!str) str = ""; _this.output.innerHTML += _this.outputfun(str) + "
"; }; - saveCode = await this.manage_scrubber(saveCode); + this.saveCode = await this.manage_scrubber(saveCode); $(this.eContainer).remove(); $(this.output).text(""); $(this.outDiv).show({ duration: 700, queue: false }); try { eval(prog); - einfo = "success"; + this.errinfo = "success"; } catch (e) { this.addErrorMessage(e); - einfo = e; + this.errinfo = e; } - // Note that, since this isn't awaited, the request may not be complete when this function returns. - this.logRunEvent({ - div_id: this.divid, - code: this.editor.getValue(), - errinfo: einfo, - lang: this.language, - to_save: saveCode, - prefix: this.pretext, - suffix: this.suffix, - partner: this.partner, - }); } }