Remove last usages of async-stacktrace from PrairieLearn app (#9982)

* Remove async-stacktrace from groupStudent.test.ts * Remove async-stacktrace from groupScoreAndSync.test.ts * Remove async-stacktrace from exam.test.ts * Remove async-stacktrace from SAML router * Remove async-stacktrace from courseEditor.test.ts * Avoid unnecessary use of async * Remove async-stacktrace from fileEditor.test.ts * Remove async-stacktrace from instructorInstanceAdminLti.js * Remove types and dependency * Address review comments
PrairieLearn · Jun 10, 2024 · ee02389 · ee02389
1 parent 39a967b
commit ee02389
Show file tree

Hide file tree

Showing 13 changed files with 331 additions and 722 deletions.
diff --git a/apps/prairielearn/package.json b/apps/prairielearn/package.json
@@ -62,7 +62,6 @@
     "archiver": "^7.0.1",
     "async": "^3.2.5",
     "async-mutex": "^0.5.0",
-    "async-stacktrace": "^0.0.2",
     "backbone": "^1.6.0",
     "better-ajv-errors": "^1.2.0",
     "blocked": "^1.3.0",

diff --git a/apps/prairielearn/src/ee/auth/saml/router.ts b/apps/prairielearn/src/ee/auth/saml/router.ts
@@ -1,4 +1,5 @@
-import ERR from 'async-stacktrace';
+import util from 'node:util';
+
 import { Router } from 'express';
 import asyncHandler from 'express-async-handler';
 import passport from 'passport';
@@ -125,22 +126,19 @@ router.post(
 
 router.get(
   '/metadata',
-  asyncHandler(async (req, res, next) => {
+  asyncHandler(async (req, res) => {
     const samlProvider = await getInstitutionSamlProvider(req.params.institution_id);
     if (!samlProvider) {
       throw new error.HttpStatusError(404, 'Institution does not support SAML authentication');
     }
 
-    strategy.generateServiceProviderMetadata(
+    const metadata = await util.promisify(strategy.generateServiceProviderMetadata)(
       req,
       samlProvider.public_key,
       samlProvider.public_key,
-      (err, metadata) => {
-        if (ERR(err, next)) return;
-        res.type('application/xml');
-        res.send(metadata);
-      },
     );
+    res.type('application/xml');
+    res.send(metadata);
   }),
 );
 

diff --git a/apps/prairielearn/src/pages/instructorInstanceAdminLti/instructorInstanceAdminLti.js b/apps/prairielearn/src/pages/instructorInstanceAdminLti/instructorInstanceAdminLti.js
@@ -1,6 +1,6 @@
 // @ts-check
-import ERR from 'async-stacktrace';
 import * as express from 'express';
+import asyncHandler from 'express-async-handler';
 import _ from 'lodash';
 
 import * as error from '@prairielearn/error';
@@ -11,71 +11,56 @@ import { getCourseOwners } from '../../lib/course.js';
 const router = express.Router();
 const sql = sqldb.loadSqlEquiv(import.meta.url);
 
-router.get('/', function (req, res, next) {
-  if (!res.locals.authz_data.has_course_permission_edit) {
-    getCourseOwners(res.locals.course.id)
-      .then((owners) => {
-        res.locals.course_owners = owners;
-        res.status(403).render(import.meta.filename.replace(/\.js$/, '.ejs'), res.locals);
-      })
-      .catch((err) => next(err));
-    return;
-  }
-  var params = {
-    course_instance_id: res.locals.course_instance.id,
-  };
+router.get(
+  '/',
+  asyncHandler(async (req, res) => {
+    if (!res.locals.authz_data.has_course_permission_edit) {
+      res.locals.course_owners = await getCourseOwners(res.locals.course.id);
+      res.status(403).render(import.meta.filename.replace(/\.js$/, '.ejs'), res.locals);
+      return;
+    }
 
-  sqldb.query(sql.lti_data, params, function (err, result) {
-    if (ERR(err, next)) return;
+    const result = await sqldb.queryAsync(sql.lti_data, {
+      course_instance_id: res.locals.course_instance.id,
+    });
     _.assign(res.locals, result.rows[0]);
 
     res.render(import.meta.filename.replace(/\.js$/, '.ejs'), res.locals);
-  });
-});
+  }),
+);
 
-router.post('/', function (req, res, next) {
-  if (!res.locals.authz_data.has_course_permission_edit) {
-    return next(new error.HttpStatusError(403, 'Access denied (must be a course Editor)'));
-  }
-  var params;
-  if (req.body.__action === 'lti_new_cred') {
-    params = {
-      key: 'K' + randomString(),
-      secret: 'S' + randomString(),
-      course_instance_id: res.locals.course_instance.id,
-    };
-    sqldb.query(sql.insert_cred, params, function (err, _result) {
-      if (ERR(err, next)) return;
-      res.redirect(req.originalUrl);
-    });
-  } else if (req.body.__action === 'lti_del_cred') {
-    params = {
-      id: req.body.lti_link_id,
-      ci_id: res.locals.course_instance.id,
-    };
-    sqldb.query(sql.delete_cred, params, function (err, _result) {
-      if (ERR(err, next)) return;
-      res.redirect(req.originalUrl);
-    });
-  } else if (req.body.__action === 'lti_link_target') {
-    var newAssessment = null;
-    if (req.body.newAssessment !== '') {
-      newAssessment = req.body.newAssessment;
+router.post(
+  '/',
+  asyncHandler(async (req, res) => {
+    if (!res.locals.authz_data.has_course_permission_edit) {
+      throw new error.HttpStatusError(403, 'Access denied (must be a course Editor)');
     }
 
-    params = {
-      assessment_id: newAssessment,
-      id: req.body.lti_link_id,
-      ci_id: res.locals.course_instance.id,
-    };
-    sqldb.query(sql.update_link, params, function (err, _result) {
-      if (ERR(err, next)) return;
+    if (req.body.__action === 'lti_new_cred') {
+      await sqldb.queryAsync(sql.insert_cred, {
+        key: 'K' + randomString(),
+        secret: 'S' + randomString(),
+        course_instance_id: res.locals.course_instance.id,
+      });
       res.redirect(req.originalUrl);
-    });
-  } else {
-    return next(new error.HttpStatusError(400, `unknown __action: ${req.body.__action}`));
-  }
-});
+    } else if (req.body.__action === 'lti_del_cred') {
+      await sqldb.queryAsync(sql.delete_cred, {
+        id: req.body.lti_link_id,
+        ci_id: res.locals.course_instance.id,
+      });
+      res.redirect(req.originalUrl);
+    } else if (req.body.__action === 'lti_link_target') {
+      await sqldb.queryAsync(sql.update_link, {
+        assessment_id: req.body.newAssessment || null,
+        id: req.body.lti_link_id,
+        ci_id: res.locals.course_instance.id,
+      });
+      res.redirect(req.originalUrl);
+    } else {
+      throw new error.HttpStatusError(400, `unknown __action: ${req.body.__action}`);
+    }
+  }),
+);
 
 export default router;
 

diff --git a/apps/prairielearn/src/tests/courseEditor.test.sql b/apps/prairielearn/src/tests/courseEditor.test.sql
@@ -15,14 +15,6 @@ ORDER BY
 LIMIT
   1;
 
--- BLOCK select_job_sequence
-SELECT
-  *
-FROM
-  job_sequences
-WHERE
-  id = $job_sequence_id;
-
 -- BLOCK select_sync_warnings_and_errors
 WITH
   course_errors AS (

diff --git a/apps/prairielearn/src/tests/courseEditor.test.ts b/apps/prairielearn/src/tests/courseEditor.test.ts
@@ -1,10 +1,8 @@
-import { exec } from 'child_process';
 import * as path from 'path';
 
-import * as async from 'async';
-import ERR from 'async-stacktrace';
 import { assert } from 'chai';
 import * as cheerio from 'cheerio';
+import { execa } from 'execa';
 import fs from 'fs-extra';
 import klaw from 'klaw';
 import fetch from 'node-fetch';
@@ -309,11 +307,8 @@ describe('test course editor', function () {
   this.timeout(20000);
 
   describe('not the example course', function () {
-    before('create test course files', function (callback) {
-      createCourseFiles((err) => {
-        if (ERR(err, callback)) return;
-        callback(null);
-      });
+    before('create test course files', async () => {
+      await createCourseFiles();
     });
 
     before('set up testing server', helperServer.before(courseDir));
@@ -433,7 +428,15 @@ function testEdit(params) {
     });
   });
 
-  waitForJobSequence(locals, 'Success');
+  describe('The job sequence', () => {
+    it('should have an id', async () => {
+      const result = await sqldb.queryOneRowAsync(sql.select_last_job_sequence, []);
+      locals.job_sequence_id = result.rows[0].id;
+    });
+    it('should complete', async () => {
+      await helperServer.waitForJobSequenceSuccess(locals.job_sequence_id);
+    });
+  });
 
   describe('validate', () => {
     it('should not have any sync warnings or errors', async () => {
@@ -443,14 +446,10 @@ function testEdit(params) {
       assert.isEmpty(results.rows);
     });
 
-    it('should pull into dev directory', function (callback) {
-      const execOptions = {
+    it('should pull into dev directory', async () => {
+      await execa('git', ['pull'], {
         cwd: courseDevDir,
         env: process.env,
-      };
-      exec(`git pull`, execOptions, (err) => {
-        if (ERR(err, callback)) return;
-        callback(null);
       });
     });
 
@@ -469,119 +468,39 @@ function testEdit(params) {
   });
 }
 
-function createCourseFiles(callback) {
-  async.series(
-    [
-      async () => await deleteCourseFiles(),
-      (callback) => {
-        const execOptions = {
-          cwd: '.',
-          env: process.env,
-        };
-        // Ensure that the default branch is master, regardless of how git
-        // is configured on the host machine.
-        exec(
-          `git -c "init.defaultBranch=master" init --bare ${courseOriginDir}`,
-          execOptions,
-          (err) => {
-            if (ERR(err, callback)) return;
-            callback(null);
-          },
-        );
-      },
-      (callback) => {
-        const execOptions = {
-          cwd: '.',
-          env: process.env,
-        };
-        exec(`git clone ${courseOriginDir} ${courseLiveDir}`, execOptions, (err) => {
-          if (ERR(err, callback)) return;
-          callback(null);
-        });
-      },
-      async () => {
-        await fs.copy(courseTemplateDir, courseLiveDir, { overwrite: false });
-      },
-      (callback) => {
-        const execOptions = {
-          cwd: courseLiveDir,
-          env: process.env,
-        };
-        exec(`git add -A`, execOptions, (err) => {
-          if (ERR(err, callback)) return;
-          callback(null);
-        });
-      },
-      (callback) => {
-        const execOptions = {
-          cwd: courseLiveDir,
-          env: process.env,
-        };
-        exec(`git commit -m "initial commit"`, execOptions, (err) => {
-          if (ERR(err, callback)) return;
-          callback(null);
-        });
-      },
-      (callback) => {
-        const execOptions = {
-          cwd: courseLiveDir,
-          env: process.env,
-        };
-        exec(`git push`, execOptions, (err) => {
-          if (ERR(err, callback)) return;
-          callback(null);
-        });
-      },
-      (callback) => {
-        const execOptions = {
-          cwd: '.',
-          env: process.env,
-        };
-        exec(`git clone ${courseOriginDir} ${courseDevDir}`, execOptions, (err) => {
-          if (ERR(err, callback)) return;
-          callback(null);
-        });
-      },
-    ],
-    (err) => {
-      if (ERR(err, callback)) return;
-      callback(null);
-    },
-  );
+async function createCourseFiles() {
+  await deleteCourseFiles();
+  // Ensure that the default branch is master, regardless of how git
+  // is configured on the host machine.
+  await execa('git', ['-c', 'init.defaultBranch=master', 'init', '--bare', courseOriginDir], {
+    cwd: '.',
+    env: process.env,
+  });
+  await execa('git', ['clone', courseOriginDir, courseLiveDir], {
+    cwd: '.',
+    env: process.env,
+  });
+  await fs.copy(courseTemplateDir, courseLiveDir, { overwrite: false });
+  await execa('git', ['add', '-A'], {
+    cwd: courseLiveDir,
+    env: process.env,
+  });
+  await execa('git', ['commit', '-m', 'initial commit'], {
+    cwd: courseLiveDir,
+    env: process.env,
+  });
+  await execa('git', ['push'], {
+    cwd: courseLiveDir,
+    env: process.env,
+  });
+  await execa('git', ['clone', courseOriginDir, courseDevDir], {
+    cwd: '.',
+    env: process.env,
+  });
 }
 
 async function deleteCourseFiles() {
   await fs.remove(courseOriginDir);
   await fs.remove(courseLiveDir);
   await fs.remove(courseDevDir);
 }
-
-function waitForJobSequence(locals, expectedResult) {
-  describe('The job sequence', function () {
-    it('should have an id', function (callback) {
-      sqldb.queryOneRow(sql.select_last_job_sequence, [], (err, result) => {
-        if (ERR(err, callback)) return;
-        locals.job_sequence_id = result.rows[0].id;
-        callback(null);
-      });
-    });
-    it('should complete', function (callback) {
-      const checkComplete = function () {
-        const params = { job_sequence_id: locals.job_sequence_id };
-        sqldb.queryOneRow(sql.select_job_sequence, params, (err, result) => {
-          if (ERR(err, callback)) return;
-          locals.job_sequence_status = result.rows[0].status;
-          if (locals.job_sequence_status === 'Running') {
-            setTimeout(checkComplete, 10);
-          } else {
-            callback(null);
-          }
-        });
-      };
-      setTimeout(checkComplete, 10);
-    });
-    it(`should have result "${expectedResult}"`, function () {
-      assert.equal(locals.job_sequence_status, expectedResult);
-    });
-  });
-}