Close #1097, wrong sign-in loses execution context b/c missing nav detection

.github/workflows/github_server.yml

@@ -157,7 +157,7 @@ jobs:
           #### Developer note: You can probably leave the rest out
           #### To learn more, see https://assemblyline.suffolklitlab.org/docs/alkiln/writing/#optional-inputs
           ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
-          # ALKILN_VERSION: 
+          ALKILN_VERSION: "screenshot"
 
       #### Developer note: Example of making an issue when tests fail
       #### that includes the text of the failure output file

.github/workflows/playground.yml

@@ -80,4 +80,4 @@ jobs:
           # want to check up on this.
           ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
           #### Developer note: You can probably leave this out
-          # ALKILN_VERSION: 
+          ALKILN_VERSION: "screenshot"

CHANGELOG.md

@@ -47,16 +47,21 @@ Format:
 
 ### Changed
 
+- On pages with sensitive answers, store the HTML of the page. The HTML excludes field values, so those sensitive answers will not be in the saved file. Still avoid taking a pic of the screen, which would reveal sensitive answers. ‼️ NEVER USE REAL USERS' ANSWERS IN ALKILN TESTS. This HTML can still reveal information about a user's answers. For example, some answers will reveal new questions. That will change the code of the revealed fields and that code will be in the HTML. See #1099
 - GitHub action release: Restored our GitHub action's default for ALKiln version to the latest version 5 again. Released first on GitHub actions. NPM release will come in time, but npm has no impact on GitHub action releases.
 
 ### Fixed
 
-- Fixed GitHub+You action outdated docassemble cli version causing Scenario timeouts.
+- Updated docassemble cli version.
+- Better detect navigation when email and password fail to sign in to a docassemble server account.
 
 ### Internal
 
-- Simplify workflow files by removing `node` env setup (leaving it to the actions)
+- Broke out the function in `Before()` and `After()` to attempt better error tracing. Goal: try to repeat this for other functions in that file.
+- Simplified workflow files by removing `node` env setup (leaving it to the actions)
 - Updated both of our actions' dependencies (the checkout, setup-node, setup-python, upload-artifacts, download-artifacts actions). Closes [#1095](https://github.com/suffolkLITLab/aLKiln/issues/1095). Once again, action related.
+- When our report phrases are missing, list all missing phrases at one time in our custom message. The cucumber message will stay the same.
+- Silenced errors from screenshots & HTML downloads when trying to provide more information about failing tests. Those records are nice to have, but not absolutely necessary, and if a server is busy puppeteer will rightly have lost execution context, preventing interactions with the page and we don't need a noisy error about it. Log any problems.
 
 ## [5.16.1] - 2026-06-01
 

lib/scope.js

@@ -2711,8 +2711,23 @@ module.exports = {
     return scope;
   },  // Ends scope.throwPageError()
 
-  take_a_screenshot: async ( scope,{ path }) => {
-    /* Takes a jpeg screenshot. Avoids destroying signatures. */
+  take_a_screenshot: async ( scope, {
+    path, disable_pic=false, disable_html=false
+  }) => {
+    /** 
+     * Saves a jpeg screenshot and page HTML to `path`. Avoids destroying
+     *     signatures.
+     * 
+     * @param {obj} scope - State and internal functions
+     * @param {obj} obj - Named arguments
+     * @param  {str} obj.path - name of path where to save the files
+     * @param  {bool} [obj.disable_pic=false] - Optional. Whether to disable a
+     *     puppeteer screenshot
+     * @param  {bool} [obj.disable_html=false] - Optional. Whether to disable
+     *     saving the HTML of the page
+     * 
+     * @returns undefined
+     * */
 
     let fullPage = true;
     let signature_elem = await scope.page.$(scope.signature_selector);
@@ -2721,28 +2736,32 @@ module.exports = {
       fullPage = false;
     }
 
-    await scope.page.screenshot({
-      path: path,
-      type: 'jpeg',
-      fullPage: fullPage,
-    });
-
-    let html_path = path;
-    if (path.endsWith(".jpg")) {
-      html_path = html_path.substring(0, html_path.length - 4) + ".html";
-    } else {
-      html_path = html_path + ".html";
+    if ( !disable_pic ){
+      await scope.page.screenshot({
+        path: path,
+        type: 'jpeg',
+        fullPage: fullPage,
+      });
     }
 
-    // Also save the HTML of the page
-    await scope.page.content().then(content => {
-      let server_url = session_vars.get_da_server_url();
-      content = content.replaceAll(/"(\/static\/.*\.css\?v=[^"]+")/g, server_url + "$1");
-      content = content.replaceAll(/"(\/static\/.*\.js\?v=[^"]+")/g, server_url + "$1");
-      content = content.replaceAll(/"(\/packagestatic\/.*\.css\?v=[^"]+")/g, server_url + "$1");
-      content = content.replaceAll(/"(\/packagestatic\/.*\.js\?v=[^"]+")/g, server_url + "$1");
-      fs.writeFileSync(html_path, content)
-    });
+    if ( !disable_html ) {
+      let html_path = path;
+      if (path.endsWith(".jpg")) {
+        html_path = html_path.substring(0, html_path.length - 4) + ".html";
+      } else {
+        html_path = html_path + ".html";
+      }
+
+      // Also save the HTML of the page
+      await scope.page.content().then(content => {
+        let server_url = session_vars.get_da_server_url();
+        content = content.replaceAll(/"(\/static\/.*\.css\?v=[^"]+")/g, server_url + "$1");
+        content = content.replaceAll(/"(\/static\/.*\.js\?v=[^"]+")/g, server_url + "$1");
+        content = content.replaceAll(/"(\/packagestatic\/.*\.css\?v=[^"]+")/g, server_url + "$1");
+        content = content.replaceAll(/"(\/packagestatic\/.*\.js\?v=[^"]+")/g, server_url + "$1");
+        fs.writeFileSync(html_path, content)
+      });
+    }
   },  // Ends scope.take_a_screenshot()
 
 
@@ -3151,7 +3170,7 @@ module.exports = {
       await scope.guard_against_missing_tap_element(scope, { elem });
 
       // Submit and see what happens
-      let winner = await scope.steps.race_sign_in_navigation( scope, { elem });
+      let winner = await scope.steps.race_sign_in_navigation( scope, { elem, login_url });
       // Add the result to the report and possibly throw errors
       if ( winner[0] === `success` ) {
         reports.addToReport( scope, {
@@ -3174,7 +3193,7 @@ module.exports = {
       }
     },  // Ends scope.steps.sign_in()
 
-    race_sign_in_navigation: async function ( scope, { elem }) {
+    race_sign_in_navigation: async function ( scope, { elem, login_url='unknown' }) {
       /** Wait for sign in navigation success or failure, or system error. */
 
       // After everything, clean up incomplete promises
@@ -3211,20 +3230,24 @@ module.exports = {
       });
 
       let click_promise = elem.click();  // MUST complete
+      let nav_promise = scope.nav_race(scope, {}).result;
       const winner = await Promise.race([
         Promise.all([ `success`, click_promise, redirect_promise ]),
         Promise.all([ `failure`, click_promise, wrong_sign_in_promise ]),
         Promise.all([ `error`, click_promise, error_promise ]),
-      ]).catch(function ( error ) {
+      ]).catch(async ( error ) => {
         let error_msg = reports.addToReport( scope, {
           type: `error`, code: `ALK0207`,
           value: `Unknown error waiting for results during sign in at ${ login_url }.`
         });
-        throw new Error( error );
+        throw error;
       });
 
       // Clean up unresolved promises
       controller.abort();
+      // Wait to finish navigating
+      // TODO: Test the winner 'error' under this circumstance
+      await nav_promise;
 
       log.debug({ code: `ALK0208`, context: `nav` },
         `Sign-in winner:`, winner
@@ -3498,14 +3521,15 @@ module.exports = {
     let scenario = scope.report.get( scope.scenario_id );
     let report = reports.getPrintableScenario( scenario );
     let all_are_included = true;
+    let missing = [];
     for ( let one_expectation of expected ) {
       if ( !report.includes( one_expectation )) {
         all_are_included = false;
-        expect( report ).to.contain( one_expectation );
+        missing.push( one_expectation );
       }
     }
 
-    return all_are_included;
+    return { all_included: all_are_included, missing };
   },  // Ends scope.reportIncludesAllExpected()
 
   reportDoesNotInclude: async function ( scope, { not_expected=[] }) {

lib/steps.js

@@ -114,8 +114,8 @@ BeforeAll(async function() {
   reports.create( scope );
 });
 
-Before(async (scenario) => {
-
+Before( beforeScenario );
+async function beforeScenario( scenario ) {
   // Start the running "progress bar" for the Scenario
   log.stdout({}, `\nScenario: ${ scenario.pickle.name }: `);
 
@@ -171,7 +171,7 @@ Before(async (scenario) => {
 
   // Reset default timeout
   scope.timeout = default_timeout;
-});
+}
 
 // Add a check for an error page before each step? After each step?
 
@@ -291,7 +291,6 @@ Given(/I go to "([^"]+)"/i, {timeout: -1}, async ( url ) => {
 
 Given(
   /I (?:sign|log) ?(?:in)?(?:on)?(?:to the server)? with(?: the email)? "([^"]+)",?(?: and)?(?: the password)? "([^"]+)"(?: SECRETs)?(?:,?(?: and)?(?: the API key)? "([^"]+)")?/i,
-  { timeout: -1 },
   async ( email, password, api_key ) => {
   /** Uses the names of environment variables (most often GitHub SECRETs) to
   *     log into an account on the user's server. Must be secure.
@@ -1333,8 +1332,8 @@ AfterStep(async function({ result }) {
   reports.outdent();
 });
 
-After(async function(scenario) {
-
+After(afterScenario);
+async function afterScenario(scenario) {
   // Log errors
   if ( scenario.result.message ) {
     log.debug({ code: `ALK0091`, context: `scenario`, },
@@ -1379,25 +1378,50 @@ After(async function(scenario) {
         reports.addToReport(scope, {
           type: `row info`,
           code: `ALK0095`,
-          value: `For security, ALKiln will avoid creating a picture of the page for this error. It's possible a secret is being used on this screen.`
+          // Discuss: Might people use secret variables to prevent any
+          // information about a screen from getting out? For example, they may
+          // have proprietary info in the HTML itself.
+          value: `For security, ALKiln will avoid creating a picture of the page for this error. It's possible a secret is being used on this screen. ALKiln will save the HTML as a file. The HTML omits the values in the fields.`
         });
-      } else {
+      }
 
+      // Could have an error if page can't load or something similar
+      try {
         // Save/download a picture of the screen that's showing during the unexpected status
         // Save one copy in the outer-most artifact folder
         let scenario_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error_on` });
         let path_outer = `${ scope.paths.artifacts }/${ scenario_filename }.jpg`;
-        await scope.take_a_screenshot( scope, { path: path_outer });
+        await scope.take_a_screenshot( scope, {
+          path: path_outer,
+          disable_pic: scope.disable_error_screenshot,
+          disable_html: false,
+        });
 
         // Save another copy in the artifact's Scenario folder
         let screenshot_name = `error_on`;
         let { id } = await scope.examinePageID( scope, 'none to match' );
         let short_id = `${ id }`.substring(0, 20);
         screenshot_name += `-${ short_id }`;
         let path_scenario = `${ scope.paths.scenario }/${ screenshot_name }.jpg`;
-        await scope.take_a_screenshot( scope, { path: path_scenario });
+        await scope.take_a_screenshot( scope, {
+          path: path_scenario,
+          disable_pic: scope.disable_error_screenshot,
+          disable_html: false,
+        });
+
+      } catch ( page_error ) {
+        // Fail silently. Our inability to take a pic shouldn't cause confusion
+        // about why a test failed
+        if ( page_error.message.lower().includes(`execution context`) ) {
+          reports.addToReport(scope, {
+            type: `row warning`,
+            code: `ALK0281`,
+            value: `ALKiln is unable to get any record of this page whatsoever, even the page HTML. Your server may be busy.`
+          });
+        }
+        log.debug({ code: `ALK0282`, level: `note`, }, page_error );
+      }
 
-      }  // ends if scope.disable_error_screenshot
     }  // ends if scope.page exists
 
     // This has to come after the security message or security message doesn't print. Not sure why.
@@ -1417,13 +1441,24 @@ After(async function(scenario) {
   // ---------------- Check internal test results ----------------
 
   if ( scope.expected_in_report && scope.expected_in_report.length > 0) {
-    let report_includes_all_expected_strings = await scope.reportIncludesAllExpected(
+    let data = await scope.reportIncludesAllExpected(
       scope,
       { expected: scope.expected_in_report }
     );
-    if ( !report_includes_all_expected_strings ) { changeable_test_status = `FAILED`; }
+    let report_includes_all_expected_strings = data.all_included;
+    if ( !report_includes_all_expected_strings ) {
+      changeable_test_status = `FAILED`;
+      let msg = `These strings are missing from the report:
+        ${ JSON.stringify(data.missing, null, 2) }
+        Instead the report had this text:
+        ${ report }`;
+      // These failure messages won't match exactly, which is a shame, but at
+      // least our devs will have more data _somewhere_ about their failure.
+      expect( report, msg ).to.contain( data.missing[0] );
+    }
     // Reset report values no matter what so they don't mess up future scenarios
     scope.expected_in_report = null;
+
   }
 
   if (scope.expected_not_in_report && scope.expected_not_in_report.length > 0) {
@@ -1605,8 +1640,7 @@ After(async function(scenario) {
   log.debug({ code: `ALK0100`, context: `scenario`, },
     `Scenario After() message:`, scenario.result.message
   );
-
-});
+}  // Ends afterScenario()
 
 AfterAll(async function() {
   // Stop collecting server response statuses

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@suffolklitlab/alkiln",
-  "version": "5.16.1",
+  "version": "5.16.1-always-html-6",
   "description": "Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.",
   "main": "lib/index.js",
   "scripts": {