From 86c67f3c9ac178556f955d82df0a4a3213e4bbce Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 24 May 2024 12:53:11 +0200 Subject: [PATCH] Refactor code execution to use async functions This refactoring is required for Sentry tracing. It ensures that the respective functions only return as soon as a code execution finished. With this approach, we can then instrument the duration of the functions, so that Sentry spans are created as desired. Co-authored-by: Jan Graichen --- app/assets/javascripts/community_solution.js | 18 +- app/assets/javascripts/editor/editor.js.erb | 12 +- app/assets/javascripts/editor/evaluation.js | 14 +- app/assets/javascripts/editor/execution.js | 95 ++++++---- .../editor/participantsupport.js.erb | 59 ++++--- app/assets/javascripts/editor/submissions.js | 165 ++++++++++-------- 6 files changed, 205 insertions(+), 158 deletions(-) diff --git a/app/assets/javascripts/community_solution.js b/app/assets/javascripts/community_solution.js index ca533e0d..d2730b19 100644 --- a/app/assets/javascripts/community_solution.js +++ b/app/assets/javascripts/community_solution.js @@ -28,13 +28,13 @@ $(document).on('turbolinks:load', function() { function submitCode(event) { const button = $(event.target) || $('#submit'); this.startSentryTransaction(button); - this.createSubmission(button, null, function (response) { - if (response.redirect) { - this.autosaveIfChanged(); - this.stopCode(event); - this.editors = []; - Turbolinks.clearCache(); - Turbolinks.visit(response.redirect); - } - }) + const submission = await this.createSubmission(button, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + if (!submission.redirect) return; + + this.autosaveIfChanged(); + this.stopCode(event); + this.editors = []; + Turbolinks.clearCache(); + Turbolinks.visit(submission.redirect); } diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index dd354df8..e9bd9e5d 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -894,12 +894,14 @@ var CodeOceanEditor = { Sentry.captureException(JSON.stringify(error, ["message", "arguments", "type", "name", "data"])); }, - showFileDialog: function (event) { + showFileDialog: async function (event) { event.preventDefault(); - this.createSubmission('#create-file', null, function (response) { - $('#code_ocean_file_context_id').val(response.id); - new bootstrap.Modal($('#modal-file')).show(); - }.bind(this)); + + const submission = await this.createSubmission('#create-file', null).catch(this.ajaxError.bind(this)); + if (!submission) return; + + $('#code_ocean_file_context_id').val(submission.id); + new bootstrap.Modal($('#modal-file')).show(); }, initializeOutputBarToggle: function () { diff --git a/app/assets/javascripts/editor/evaluation.js b/app/assets/javascripts/editor/evaluation.js index c0e0bb0e..8f7a7fcd 100644 --- a/app/assets/javascripts/editor/evaluation.js +++ b/app/assets/javascripts/editor/evaluation.js @@ -8,17 +8,19 @@ CodeOceanEditorEvaluation = { * Scoring-Functions */ scoreCode: function (event) { + event.preventDefault(); const cause = $('#assess'); this.startSentryTransaction(cause); - event.preventDefault(); this.stopCode(event); this.clearScoringOutput(); $('#submit').addClass("d-none"); - this.createSubmission(cause, null, function (submission) { - this.showSpinner($('#assess')); - $('#score_div').removeClass('d-none'); - this.initializeSocketForScoring(submission.id); - }.bind(this)); + + const submission = await this.createSubmission(cause, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + + this.showSpinner($('#assess')); + $('#score_div').removeClass('d-none'); + await this.socketScoreCode(submission.id); }, handleScoringResponse: function (results) { diff --git a/app/assets/javascripts/editor/execution.js b/app/assets/javascripts/editor/execution.js index 0d4287c8..1622ee98 100644 --- a/app/assets/javascripts/editor/execution.js +++ b/app/assets/javascripts/editor/execution.js @@ -3,7 +3,7 @@ CodeOceanEditorWebsocket = { // Replace `http` with `ws` for the WebSocket connection. This also works with `https` and `wss`. webSocketProtocol: window.location.protocol.replace(/^http/, 'ws').split(':')[0], - initializeSocket: function(urlHelper, params, closeCallback) { + runSocket: function(urlHelper, params, setupFunction) { // 1. Specify the protocol for all URLs to generate params.protocol = this.webSocketProtocol; params._options = true; @@ -32,48 +32,73 @@ CodeOceanEditorWebsocket = { // 4. Connect to the given URL. this.websocket = new CommandSocket(url, - function (evt) { - this.resetOutputTab(); - }.bind(this) + function (evt) { + this.resetOutputTab(); + }.bind(this) ); + + // Attach custom handlers for messages received. + setupFunction(this.websocket); + CodeOceanEditorWebsocket.websocket = this.websocket; - this.websocket.onError(this.showWebsocketError.bind(this)); - this.websocket.onClose(function(span, callback){ - span?.finish(); - if(callback != null){ - callback(); - } - }.bind(this, span, closeCallback)); + + // Create and return a new Promise. It will only resolve (or fail) once the connection has ended. + return new Promise((resolve, reject) => { + this.websocket.onError(this.showWebsocketError.bind(this)); + + // Remove event listeners for Promise handling. + // This is especially useful in case of an error, where a `close` event might follow the `error` event. + const teardown = () => { + this.websocket.websocket.removeEventListener(closeListener); + this.websocket.websocket.removeEventListener(errorListener); + }; + + // We are using event listeners (and not `onError` or `onClose`) here, since these listeners should never be overwritten. + // With `onError` or `onClose`, a new assignment would overwrite a previous one. + const closeListener = this.websocket.websocket.addEventListener('close', () => { + span?.finish(); + resolve(); + teardown(); + }); + const errorListener = this.websocket.websocket.addEventListener('error', (error) => { + reject(error); + teardown(); + this.websocket.killWebSocket(); // In case of error, ensure we always close the connection. + }); + }); }, - initializeSocketForTesting: function(submissionID, filename) { - this.initializeSocket(Routes.test_submission_url, {id: submissionID, filename: filename}); - this.websocket.on('default',this.handleTestResponse.bind(this)); - this.websocket.on('exit', this.handleExitCommand.bind(this)); + socketTestCode: function(submissionID, filename) { + return this.runSocket(Routes.test_submission_url, {id: submissionID, filename: filename}, (websocket) => { + websocket.on('default', this.handleTestResponse.bind(this)); + websocket.on('exit', this.handleExitCommand.bind(this)); + }); }, - initializeSocketForScoring: function(submissionID) { - this.initializeSocket(Routes.score_submission_url, {id: submissionID}, function() { - $('#assess').one('click', this.scoreCode.bind(this)) - }.bind(this)); - this.websocket.on('default',this.handleScoringResponse.bind(this)); - this.websocket.on('hint', this.showHint.bind(this)); - this.websocket.on('exit', this.handleExitCommand.bind(this)); - this.websocket.on('status', this.showStatus.bind(this)); + socketScoreCode: function(submissionID) { + return this.runSocket(Routes.score_submission_url, {id: submissionID}, (websocket) => { + websocket.on('default', this.handleScoringResponse.bind(this)); + websocket.on('hint', this.showHint.bind(this)); + websocket.on('exit', this.handleExitCommand.bind(this)); + websocket.on('status', this.showStatus.bind(this)); + }).then(() => { + $('#assess').one('click', this.scoreCode.bind(this)); + }); }, - initializeSocketForRunning: function(submissionID, filename) { - this.initializeSocket(Routes.run_submission_url, {id: submissionID, filename: filename}); - this.websocket.on('input',this.showPrompt.bind(this)); - this.websocket.on('write', this.printWebsocketOutput.bind(this)); - this.websocket.on('clear', this.clearOutput.bind(this)); - this.websocket.on('turtle', this.handleTurtleCommand.bind(this)); - this.websocket.on('turtlebatch', this.handleTurtlebatchCommand.bind(this)); - this.websocket.on('render', this.printWebsocketOutput.bind(this)); - this.websocket.on('exit', this.handleExitCommand.bind(this)); - this.websocket.on('status', this.showStatus.bind(this)); - this.websocket.on('hint', this.showHint.bind(this)); - this.websocket.on('files', this.prepareFileDownloads.bind(this)); + socketRunCode: function(submissionID, filename) { + return this.runSocket(Routes.run_submission_url, {id: submissionID, filename: filename}, (websocket) => { + websocket.on('input', this.showPrompt.bind(this)); + websocket.on('write', this.printWebsocketOutput.bind(this)); + websocket.on('clear', this.clearOutput.bind(this)); + websocket.on('turtle', this.handleTurtleCommand.bind(this)); + websocket.on('turtlebatch', this.handleTurtlebatchCommand.bind(this)); + websocket.on('render', this.printWebsocketOutput.bind(this)); + websocket.on('exit', this.handleExitCommand.bind(this)); + websocket.on('status', this.showStatus.bind(this)); + websocket.on('hint', this.showHint.bind(this)); + websocket.on('files', this.prepareFileDownloads.bind(this)); + }); }, handleExitCommand: function() { diff --git a/app/assets/javascripts/editor/participantsupport.js.erb b/app/assets/javascripts/editor/participantsupport.js.erb index 988c6d38..a782ebe9 100644 --- a/app/assets/javascripts/editor/participantsupport.js.erb +++ b/app/assets/javascripts/editor/participantsupport.js.erb @@ -114,41 +114,42 @@ CodeOceanEditorRequestForComments = { questionElement.prop("disabled", true); $('#closeAskForCommentsButton').addClass('d-none'); - var exercise_id = editor.data('exercise-id'); - var file_id = $('.editor').data('id'); - var question = questionElement.val(); + const exercise_id = editor.data('exercise-id'); + const file_id = $('.editor').data('id'); + const question = questionElement.val(); - var createRequestForComments = function (submission) { - this.showSpinner($('#askForCommentsButton')); - $.ajax({ - method: 'POST', - url: Routes.request_for_comments_path(), - data: { - request_for_comment: { - exercise_id: exercise_id, - file_id: file_id, - submission_id: submission.id, - question: question - } + const submission = await this.createSubmission(cause, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + + this.showSpinner($('#askForCommentsButton')); + + const response = await $.ajax({ + method: 'POST', + url: Routes.request_for_comments_path(), + data: { + request_for_comment: { + exercise_id: exercise_id, + file_id: file_id, + submission_id: submission.id, + question: question } - }).done(function() { - // trigger a run - this.runSubmission.call(this, submission); - $.flash.success({text: $('#askForCommentsButton').data('message-success')}); - }.bind(this)).fail(this.ajaxError.bind(this)) - .always(function () { - bootstrap.Modal.getInstance($('#comment-modal')).hide(); - this.hideSpinner(); - $('#question').prop("disabled", false).val(''); - $('#closeAskForCommentsButton').removeClass('d-none'); - $('#askForCommentsButton').one('click', this.requestComments.bind(this)); - }.bind(this)); - }; + } + }).catch(this.ajaxError.bind(this)); + + bootstrap.Modal.getInstance($('#comment-modal')).hide(); + this.hideSpinner(); + $('#question').prop("disabled", false).val(''); + $('#closeAskForCommentsButton').removeClass('d-none'); + $('#askForCommentsButton').one('click', this.requestComments.bind(this)); - this.createSubmission(cause, null, createRequestForComments.bind(this)); // we disabled the button to prevent that the user spams RFCs, but decided against this now. //var button = $('#requestComments'); //button.prop('disabled', true); + + if (response) { + await this.runSubmission(submission); + $.flash.success({text: $('#askForCommentsButton').data('message-success')}); + } } }; diff --git a/app/assets/javascripts/editor/submissions.js b/app/assets/javascripts/editor/submissions.js index f976c23f..909d3f50 100644 --- a/app/assets/javascripts/editor/submissions.js +++ b/app/assets/javascripts/editor/submissions.js @@ -6,10 +6,10 @@ CodeOceanEditorSubmissions = { /** * Submission-Creation */ - createSubmission: function (initiator, filter, callback) { + createSubmission: async function (initiator, filter) { const editor = $('#editor'); this.showSpinner(initiator); - var url = $(initiator).data('url') || editor.data('submissions-url'); + const url = $(initiator).data('url') || editor.data('submissions-url'); if (url === undefined) { const data = { @@ -19,25 +19,29 @@ CodeOceanEditorSubmissions = { Sentry.captureException(JSON.stringify(data)); return; } - var jqxhr = this.ajax({ - data: { - submission: { - cause: $(initiator).data('cause') || $(initiator).prop('id'), - exercise_id: editor.data('exercise-id') || $(initiator).data('exercise-id'), - files_attributes: (filter || _.identity)(this.collectFiles()) - } - }, - dataType: 'json', - method: $(initiator).data('http-method') || 'POST', - url: url, - }); - jqxhr.always(this.hideSpinner.bind(this)); - jqxhr.done(this.createSubmissionCallback.bind(this)); - if(callback != null){ - jqxhr.done(callback.bind(this)); - } - jqxhr.fail(this.ajaxError.bind(this)); + try { + const response = await this.ajax({ + data: { + submission: { + cause: $(initiator).data('cause') || $(initiator).prop('id'), + exercise_id: editor.data('exercise-id') || $(initiator).data('exercise-id'), + files_attributes: (filter || _.identity)(this.collectFiles()) + } + }, + dataType: 'json', + method: $(initiator).data('http-method') || 'POST', + url: url, + }); + this.hideSpinner(); + this.createSubmissionCallback(response); + return response; + } catch (error) { + this.hideSpinner(); + + // We require the callee to handle this error, e.g., through `this.ajaxError(error)` + throw error; + } }, collectFiles: function() { @@ -81,34 +85,42 @@ CodeOceanEditorSubmissions = { /** * File-Management */ - destroyFile: function() { - this.createSubmission($('#destroy-file'), function(files) { + destroyFile: async function() { + const submission = await this.createSubmission($('#destroy-file'), function(files) { return _.reject(files, function(file) { return file.file_id === CodeOceanEditor.active_file.id; }); - }, window.CodeOcean.refresh); + }).catch(this.ajaxError.bind(this)); + if(!submission) return; + + window.CodeOcean.refresh(); }, - downloadCode: function(event) { + downloadCode: async function(event) { event.preventDefault(); - this.createSubmission('#download', null,function(submission) { - // to download just a single file, use the following url - // window.location = Routes.download_file_submission_url(submission.id, CodeOceanEditor.active_file.filename); - window.location = Routes.download_submission_url(submission.id); - }); + + const submission = await this.createSubmission('#download', null).catch(this.ajaxError.bind(this)); + if(!submission) return; + + // to download just a single file, use the following url + // window.location = Routes.download_file_submission_url(submission.id, CodeOceanEditor.active_file.filename); + window.location = Routes.download_submission_url(submission.id); }, resetCode: function(initiator, onlyActiveFile = false) { this.startSentryTransaction(initiator); this.showSpinner(initiator); - this.ajax({ + + const response = await this.ajax({ method: 'GET', url: $('#start-over').data('url') || $('#start-over-active-file').data('url') - }).done(function(response) { - this.hideSpinner(); - App.synchronized_editor?.reset_content(response); - this.setEditorContent(response, onlyActiveFile); - }.bind(this)); + }).catch(this.ajaxError.bind(this)); + + this.hideSpinner(); + + if (!response) return; + App.synchronized_editor?.reset_content(response); + this.setEditorContent(response, onlyActiveFile); }, setEditorContent: function(new_content, onlyActiveFile = false) { @@ -126,68 +138,73 @@ CodeOceanEditorSubmissions = { }, renderCode: function(event) { + event.preventDefault(); const cause = $('#render'); this.startSentryTransaction(cause); - event.preventDefault(); - if (cause.is(':visible')) { - this.createSubmission(cause, null, function (submission) { - if (submission.render_url === undefined) return; + if (!cause.is(':visible')) return; - const active_file = CodeOceanEditor.active_file.filename; - const desired_file = submission.render_url.filter(hash => hash.filepath === active_file); - const url = desired_file[0].url; - // Allow to open the new tab even in Safari. - // See: https://stackoverflow.com/a/70463940 - setTimeout(() => { - var pop_up_window = window.open(url, '_blank'); - if (pop_up_window) { - pop_up_window.onerror = function (message) { - this.clearOutput(); - this.printOutput({ - stderr: message - }, true, 0); - this.sendError(message, submission.id); - this.showOutputBar(); - }; - } - }) - }); - } + const submission = await this.createSubmission(cause, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + if (submission.render_url === undefined) return; + + const active_file = CodeOceanEditor.active_file.filename; + const desired_file = submission.render_url.filter(hash => hash.filepath === active_file); + const url = desired_file[0].url; + + // Allow to open the new tab even in Safari. + // See: https://stackoverflow.com/a/70463940 + setTimeout(() => { + var pop_up_window = window.open(url, '_blank'); + if (pop_up_window) { + pop_up_window.onerror = function (message) { + this.clearOutput(); + this.printOutput({ + stderr: message + }, true, 0); + this.sendError(message, submission.id); + this.showOutputBar(); + }; + } + }) }, /** * Execution-Logic */ runCode: function(event) { + event.preventDefault(); const cause = $('#run'); this.startSentryTransaction(cause); - event.preventDefault(); this.stopCode(event); - if (cause.is(':visible')) { - this.createSubmission(cause, null, this.runSubmission.bind(this)); - } + if (!cause.is(':visible')) return; + + const submission = await this.createSubmission(cause, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + + await this.runSubmission(submission); }, - runSubmission: function (submission) { + runSubmission: async function (submission) { //Run part starts here this.running = true; this.showSpinner($('#run')); $('#score_div').addClass('d-none'); this.toggleButtonStates(); - this.initializeSocketForRunning(submission.id, CodeOceanEditor.active_file.filename); + await this.socketRunCode(submission.id, CodeOceanEditor.active_file.filename); }, testCode: function(event) { + event.preventDefault(); const cause = $('#test'); this.startSentryTransaction(cause); - event.preventDefault(); - if (cause.is(':visible')) { - this.createSubmission(cause, null, function(submission) { - this.showSpinner($('#test')); - $('#score_div').addClass('d-none'); - this.initializeSocketForTesting(submission.id, CodeOceanEditor.active_file.filename); - }.bind(this)); - } + if (!cause.is(':visible')) return; + + const submission = await this.createSubmission(cause, null).catch(this.ajaxError.bind(this)); + if (!submission) return; + + this.showSpinner($('#test')); + $('#score_div').addClass('d-none'); + await this.socketTestCode(submission.id, CodeOceanEditor.active_file.filename); }, /** @@ -216,6 +233,6 @@ CodeOceanEditorSubmissions = { autosave: function () { clearTimeout(this.autosaveTimer); this.autosaveTimer = null; - this.createSubmission($('#autosave'), null); + this.createSubmission($('#autosave'), null).catch(this.ajaxError.bind(this)); } };