Various improvements to the /wattsi endpoint

In the course of working on adding a full /html-build endpoint, a few improvements came up which we can apply ahead of time to just the /wattsi endpoint: * Improve README documentation. * Abstract out the code for turning query parameters into boolean arguments. * Use more modern JS features. * Put temporary directories inside the OS temp directory. * Add a new helper for spawning a command-line program while capturing its output. This fixes #3. * Perform cleanup file deletions in parallel and log errors if they occur while deleting. * Move from deprecated --production npm install argument to modern --omit=dev.
whatwg · Aug 31, 2023 · 549e077 · 549e077
1 parent c660e08
commit 549e077
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 57 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -1,4 +1,4 @@
-FROM node:18.17.0-bookworm-slim
+FROM node:18.17.1-bookworm-slim
 RUN apt-get update && \
     apt-get install --yes --no-install-recommends p7zip-full && \
     rm -rf /var/lib/apt/lists/*
@@ -9,7 +9,7 @@ WORKDIR /app
 
 COPY . .
 
-RUN npm install --production
+RUN npm install --omit=dev
 
 ENV PORT=3000
 

diff --git a/README.md b/README.md
@@ -2,46 +2,43 @@
 
 This app is a build server to allow you to run [Wattsi](https://github.com/whatwg/wattsi) without having to actually install it locally. Which is really useful, since not everyone has a Free Pascal compiler lying around.
 
-Currently it is hosted on build.whatwg.org. You can use it as follows:
+Currently it is hosted on build.whatwg.org.Currently it is hosted on build.whatwg.org.
 
-1. Get a local copy of `html.json` from <https://raw.githubusercontent.com/w3c/mdn-spec-links/master/html.json>.
-1. Get the HTML spec source file `source` by checking out [whatwg/html](https://github.com/whatwg/html).
-1. Run the following command:
+## Endpoints
 
-   ```sh
-   curl https://build.whatwg.org/wattsi --verbose \
-        --form build=default \
-        --form sha=d3adb33f \
-        --form source=@source \
-        --form [email protected] \
-        --output output.zip
-   ```
+### `/wattsi`
 
-The result will be a ZIP file containing the output of Wattsi! It will also contain an `output.txt` file containing the stdout output of Wattsi, which might contain warnings or similar things you want to check out.
+The `/wattsi` endpoint accepts POSTs with the following request body fields:
 
-(NOTE: if you get a non-200 response, the resulting zip file will actually be a text file containing some error text. To account for this, you may want to use [a more complicated incantation](https://github.com/whatwg/html-build/blob/18bdae0a716c47e326abb6312357fcc8d696a7f2/build.sh#L655-L677).)
+- `source`, a file, which you can get from [whatwg/html](https://github.com/whatwg/html)
+- `mdn`, a file, which you can get from <https://raw.githubusercontent.com/w3c/mdn-spec-links/master/html.json>
+- `sha`, a string, the Git commit hash of the whatwg/html repository
+- `build`, a string, either `"default"` or `"review"`
 
-## Other Features
+You can also send the following query string parameters, which correspond to the same-named Wattsi options:
 
-You can send the query string parameter `quiet` to pass the `--quiet` option to Wattsi.
+- `quiet`
+- `single-page-only`
 
-You can send the query string paramter `single-page-only` to pass the `--single-page-only` option to Wattsi.
+If the resulting status code is 200, the result will be a ZIP file containing the output, as well as an `output.txt` containing the stdout/stderr output. If the resulting status code is 400, the body text will be the error message.
 
 The response will have a header, `Wattsi-Exit-Code`, which gives the exit code of Wattsi. This will always be `0` for a 200 OK response, but a 400 Bad Request could give a variety of different values, depending on how Wattsi failed.
 
-You can hit the `/version` endpoint with a GET to check to see if the server is working. It should return the `text/plain` response of the latest-deployed Git commit SHA.
+### `/version`
+
+This endpoint responds to GET requests so you can check to see if the server is working. It returns a `text/plain` response of the latest-deployed Git commit SHA.
 
 ## Server Development Info
 
 This server requires the following to run:
 
-- [Node.js](https://nodejs.org/) 11.4.0 or later
+- [Node.js](https://nodejs.org/) 18.17.1 or later
 - [7zip](http://www.7-zip.org/) in your path as `7za`
 - And, of course, [Wattsi](https://github.com/whatwg/wattsi), in your `$PATH` as `wattsi`
 
 It will expose itself on the port given by the `$PORT` environment variable.
 
-To set up the server remember to do `npm install --production`. Then, to start it running, just do `npm start`.
+To set up the server remember to do `npm install --omit=dev`. Then, to start it running, just do `npm start`.
 
 Alternately, you can use Docker:
 

diff --git a/lib/app.js b/lib/app.js
@@ -1,12 +1,14 @@
 "use strict";
-const Koa = require("koa");
-const KoaRouter = require("koa-router");
-const { koaBody } = require("koa-body");
 const path = require("path");
+const os = require("os");
+const childProcess = require("child_process");
 const { promisify } = require("util");
-const execFile = promisify(require("child_process").execFile);
+const execFile = promisify(childProcess.execFile);
 const { createReadStream } = require("fs");
-const { mkdir, rm, unlink, writeFile } = require("fs").promises;
+const { mkdir, rm, writeFile } = require("fs").promises;
+const Koa = require("koa");
+const KoaRouter = require("koa-router");
+const { koaBody } = require("koa-body");
 const finished = require("finished");
 
 const app = new Koa();
@@ -28,58 +30,50 @@ const bodyParser = koaBody({
 });
 
 router.post("/wattsi", bodyParser, async ctx => {
-  const quiet = "quiet" in ctx.request.query;
-  const singlePageOnly = "single-page-only" in ctx.request.query;
+  const booleanArgs = booleanArgsFromQuery(ctx.request.query, ["quiet", "single-page-only"]);
 
   const sha = ctx.request.body.sha || "(sha not provided)";
   const buildType = ctx.request.body.build || "default";
 
-  const sourceFilePath = (ctx.request.files.source && ctx.request.files.source.filepath) ||
-                         ctx.throw(400, "Expected a source file");
-  const mdnFilePath = (ctx.request.files.mdn && ctx.request.files.mdn.filepath) ||
-                      ctx.throw(400, "Expected a mdn file");
+  const sourceFilePath = ctx.request.files.source?.filepath ?? ctx.throw(400, "Expected a source file");
+  const mdnFilePath = ctx.request.files.mdn?.filepath ?? ctx.throw(400, "Expected a mdn file");
 
-  const outDirectory = randomDirectoryName();
+  const outDirectory = newTempDirectoryName();
   await mkdir(outDirectory, { recursive: true });
 
-  const args = [sourceFilePath, sha, outDirectory, buildType, mdnFilePath];
-  if (singlePageOnly) {
-    args.unshift("--single-page-only");
-  }
-  if (quiet) {
-    args.unshift("--quiet");
-  }
+  const args = [sourceFilePath, sha, outDirectory, buildType, mdnFilePath, ...booleanArgs];
 
   try {
     try {
       console.log(`Running wattsi ${args.join(" ")}`);
-      const result = await execFile("wattsi", args);
+      const result = await promisedSpawnWhileCapturingOutput("wattsi", args);
 
       const outputFile = path.join(outDirectory, "output.txt");
-      await writeFile(outputFile, `${result.stdout}\n\n${result.stderr}`, { encoding: "utf-8" });
+      await writeFile(outputFile, result, { encoding: "utf-8" });
       console.log(`  wattsi succeeded`);
     } catch (e) {
-      if (e.stdout) {
-        e.message = `${e.stdout}\n\n${e.stderr}`;
-      }
-      console.log(`  wattsi or file-writing failed: ${e.code}`);
+      const errorBody = e.output ?? e.stack;
+      console.log(`  html-build or file-writing failed:`);
+      console.log(errorBody);
       const headers = typeof e.code === "number" ? { "Wattsi-Exit-Code": e.code } : {};
-      ctx.throw(400, e.message, { headers });
+      ctx.throw(400, errorBody, { headers });
     }
 
     ctx.response.set("Wattsi-Exit-Code", "0");
     const zipFilePath = `${outDirectory}.zip`;
     console.log(`  zipping result`);
-    await execFile("7za", ["a", "-tzip", "-r", zipFilePath, `./${outDirectory}/*`]);
+    await execFile("7za", ["a", "-tzip", "-r", zipFilePath, `${outDirectory}/*`]);
     console.log(`  zipping succeeded`);
 
     ctx.response.type = "application/zip";
     ctx.response.body = createReadStream(zipFilePath);
 
-    finished(ctx, () => unlink(zipFilePath));
+    finished(ctx, () => rm(zipFilePath));
   } finally {
-    await removeAllFiles(ctx);
-    await rm(outDirectory, { recursive: true });
+    await cleanupLoggingRejections([
+      ...requestFinalRemovalPromises(ctx.request),
+      rm(outDirectory, { recursive: true })
+    ]);
   }
 });
 
@@ -88,11 +82,57 @@ app
   .use(router.allowedMethods())
   .listen(process.env.PORT);
 
-function randomDirectoryName() {
-  return Math.random().toString(36).slice(2);
+function newTempDirectoryName() {
+  return path.resolve(os.tmpdir(), Math.random().toString(36).slice(2));
+}
+
+function requestFinalRemovalPromises(request) {
+  const filePaths = Object.values(request.files).map(file => file.filepath);
+  return filePaths.map(filePath => rm(filePath));
+}
+
+function booleanArgsFromQuery(query, possibleArgs) {
+  return possibleArgs.filter(arg => arg in query).map(arg => `--${arg}`);
+}
+
+function promisedSpawnWhileCapturingOutput(...args) {
+  return new Promise((resolve, reject) => {
+    const subprocess = childProcess.spawn(...args);
+
+    let output = "";
+    subprocess.stdout.on("data", data => {
+      output += data;
+    });
+    subprocess.stderr.on("data", data => {
+      output += data;
+    });
+
+    subprocess.on("close", code => {
+      if (code !== 0) {
+        const error = new Error("Process returned nonzero exit code");
+        error.code = code;
+        error.output = output;
+        reject(error);
+      } else {
+        resolve(output);
+      }
+    });
+
+    subprocess.on("error", () => {
+      reject(new Error("Process failed with error event"));
+    });
+  });
 }
 
-function removeAllFiles(ctx) {
-  const filePaths = Object.values(ctx.request.files).map(file => file.filepath);
-  return Promise.all(filePaths.map(filePath => unlink(filePath)));
+async function cleanupLoggingRejections(promises) {
+  const results = await Promise.allSettled(promises);
+  const rejections = results.filter(result => result.status === "rejected");
+
+  if (rejections.length > 0) {
+    const plural = rejections.length === 1 ? "" : "s";
+    console.log(`  Cleanup error${plural}:`);
+    for (const result of rejections) {
+      console.log(`  ${result.reason.stack}`);
+    }
+  }
 }