feat(env-options): env-options conveniences for common cases (#1710)

packages/env-options/README.md

@@ -34,24 +34,57 @@ optionally holding a property with the same name as the option,
 whose value is the configuration setting of that option.
 
 ```js
-import { makeEnvironmentCaptor } from '@endo/env-options';
-const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis);
+import { getEnvironmentOption } from '@endo/env-options';
 const FooBarOption = getEnvironmentOption('FOO_BAR', 'absent');
 ```
 
 The first argument to `getEnvironmentOption` is the name of the option.
 The value of `FooBarOption` would then be the value of
 `globalThis.process.env.FOO_BAR`, if present.
-If setting is either absent or `undefined`, the default `'absent'`
-would be used instead.
+If value is either absent or `undefined`, the second argument,
+such as `'absent'`, would be used instead.
 
 In either case, reflecting Unix environment variable expectations,
 the resulting setting must be a string.
 This restriction also helps ensure that this channel is used only to pass data,
 not authority beyond the ability to read this global state.
 
-The `makeEnvironmentCaptor` function also returns a
-`getCapturedEnvironmentOptionNames` function for use to give feedback about
+```js
+const ENABLED =
+  getEnvironmentOption('TRACK_TURNS', 'disabled', ['enabled']) === 'enabled';
+```
+
+`getEnvironmentOption` also takes an optional third argument, which if present
+is an exhaustive list of allowed strings other than the default. If present
+and the actual environment option is neither the default nor one of these
+allowed strings, then an error is thrown explaining the problem.
+
+```js
+const DEBUG_VALUES = getEnvironmentOptionsList('DEBUG');
+const DEBUG_AGORIC = environmentOptionsListHas('DEBUG', 'agoric');
+```
+
+Another common convention is for the value of an option to be a
+comma (`','`) separated list of strings. `getEnvironmentOptionsList` will
+return this list, or an empty list if the option is absent.
+`environmentOptionsListHas` will test if this list contains a specific
+value, or return false if the option is absent.
+
+(Compat note: https://github.com/Agoric/agoric-sdk/issues/8096 explains that
+for `DEBUG` specifically, some existing uses split on colon (`':'`) rather
+than comma. Once these are fixed, then these uses can be switched to use
+`getEnvironmentOptionsList` or `environmentOptionsListHas`.)
+
+## Tracking used option names
+
+The `'@endo/env-options'` module also exports a lower-level
+`makeEnvironmentCaptor` that you can apply to whatever object you wish to treat
+as a global(having a "process" property with its own "env" record),
+such as the global of another compartment. It returns an entagled
+pair of a `getEnvironmentOption` function as above, and a
+`getCapturedEnvironmentOptionNames` function that returns an array of
+the option names used by that `getEnvironmentOption` function. This is
+useful to give feedback about
 which environment variables were actually read, for diagnostic purposes.
 For example, the
 ses-shim `lockdown` once contained code such as the following, to explain which
@@ -61,6 +94,8 @@ environment variables were read to provide `lockdown` settings.
 import { makeEnvironmentCaptor } from '@endo/env-options';
 const {
   getEnvironmentOption,
+  getEnvironmentOptionsList,
+  environmentOptionsListHas,
   getCapturedEnvironmentOptionNames,
 } = makeEnvironmentCaptor(globalThis);
 ...

packages/env-options/src/env-options.js

@@ -1,3 +1,4 @@
+/* global globalThis */
 // @ts-check
 
 // `@endo/env-options` needs to be imported quite early, and so should
@@ -17,6 +18,8 @@ const uncurryThis =
   (receiver, ...args) =>
     apply(fn, receiver, args);
 const arrayPush = uncurryThis(Array.prototype.push);
+const arrayIncludes = uncurryThis(Array.prototype.includes);
+const stringSplit = uncurryThis(String.prototype.split);
 
 const q = JSON.stringify;
 
@@ -37,8 +40,10 @@ const Fail = (literals, ...args) => {
  * the environment variables that were captured.
  *
  * @param {object} aGlobal
+ * @param {boolean} [dropNames] Defaults to false. If true, don't track
+ * names used.
  */
-export const makeEnvironmentCaptor = aGlobal => {
+export const makeEnvironmentCaptor = (aGlobal, dropNames = false) => {
   const capturedEnvironmentOptionNames = [];
 
   /**
@@ -47,13 +52,17 @@ export const makeEnvironmentCaptor = aGlobal => {
    *
    * @param {string} optionName
    * @param {string} defaultSetting
+   * @param {string[]} [optOtherValues]
+   * If provided, the option value must be included or match `defaultSetting`.
    * @returns {string}
    */
-  const getEnvironmentOption = (optionName, defaultSetting) => {
-    // eslint-disable-next-line @endo/no-polymorphic-call
+  const getEnvironmentOption = (
+    optionName,
+    defaultSetting,
+    optOtherValues = undefined,
+  ) => {
     typeof optionName === 'string' ||
       Fail`Environment option name ${q(optionName)} must be a string.`;
-    // eslint-disable-next-line @endo/no-polymorphic-call
     typeof defaultSetting === 'string' ||
       Fail`Environment option default setting ${q(
         defaultSetting,
@@ -66,7 +75,9 @@ export const makeEnvironmentCaptor = aGlobal => {
       (typeof globalProcess === 'object' && globalProcess.env) || undefined;
     if (typeof globalEnv === 'object') {
       if (optionName in globalEnv) {
-        arrayPush(capturedEnvironmentOptionNames, optionName);
+        if (!dropNames) {
+          arrayPush(capturedEnvironmentOptionNames, optionName);
+        }
         const optionValue = globalEnv[optionName];
         // eslint-disable-next-line @endo/no-polymorphic-call
         typeof optionValue === 'string' ||
@@ -78,15 +89,49 @@ export const makeEnvironmentCaptor = aGlobal => {
         setting = optionValue;
       }
     }
+    optOtherValues === undefined ||
+      setting === defaultSetting ||
+      arrayIncludes(optOtherValues, setting) ||
+      Fail`Unrecognized ${q(optionName)} value ${q(
+        setting,
+      )}. Expected one of ${q([defaultSetting, ...optOtherValues])}`;
     return setting;
   };
   freeze(getEnvironmentOption);
 
+  /**
+   * @param {string} optionName
+   * @returns {string[]}
+   */
+  const getEnvironmentOptionsList = optionName => {
+    const option = getEnvironmentOption(optionName, '');
+    return freeze(option === '' ? [] : stringSplit(option, ','));
+  };
+  freeze(getEnvironmentOptionsList);
+
+  const environmentOptionsListHas = (optionName, element) =>
+    arrayIncludes(getEnvironmentOptionsList(optionName), element);
+
   const getCapturedEnvironmentOptionNames = () => {
     return freeze([...capturedEnvironmentOptionNames]);
   };
   freeze(getCapturedEnvironmentOptionNames);
 
-  return freeze({ getEnvironmentOption, getCapturedEnvironmentOptionNames });
+  return freeze({
+    getEnvironmentOption,
+    getEnvironmentOptionsList,
+    environmentOptionsListHas,
+    getCapturedEnvironmentOptionNames,
+  });
 };
 freeze(makeEnvironmentCaptor);
+
+/**
+ * For the simple case, where the global in question is `globalThis` and no
+ * reporting of option names is desired.
+ */
+export const {
+  getEnvironmentOption,
+  getEnvironmentOptionsList,
+  environmentOptionsListHas,
+} = makeEnvironmentCaptor(globalThis, true);

packages/eventual-send/src/track-turns.js

@@ -1,7 +1,5 @@
 /* global globalThis */
-import { makeEnvironmentCaptor } from '@endo/env-options';
-
-const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis);
+import { getEnvironmentOption } from '@endo/env-options';
 
 // NOTE: We can't import these because they're not in scope before lockdown.
 // import { assert, details as X, Fail } from '@agoric/assert';
@@ -20,15 +18,18 @@ let hiddenCurrentEvent = 0;
 const DEBUG = getEnvironmentOption('DEBUG', '');
 
 // Turn on if you seem to be losing error logging at the top of the event loop
+//
+// TODO This use of colon (`':'`) as a separator is a bad legacy convention.
+// It should be comma (`','`). Once we can switch it to comma, then use the
+// following commented out definition of `VERBOSE` instead.
+// const VERBOSE = environmentOptionsListHas('DEBUG', 'track-turns');
+// See https://github.com/Agoric/agoric-sdk/issues/8096
 const VERBOSE = DEBUG.split(':').includes('track-turns');
 
 // Track-turns is disabled by default and can be enabled by an environment
 // option.
-const TRACK_TURNS = getEnvironmentOption('TRACK_TURNS', 'disabled');
-if (TRACK_TURNS !== 'enabled' && TRACK_TURNS !== 'disabled') {
-  throw TypeError(`unrecognized TRACK_TURNS ${JSON.stringify(TRACK_TURNS)}`);
-}
-const ENABLED = (TRACK_TURNS || 'disabled') === 'enabled';
+const ENABLED =
+  getEnvironmentOption('TRACK_TURNS', 'disabled', ['enabled']) === 'enabled';
 
 // We hoist the following functions out of trackTurns() to discourage the
 // closures from holding onto 'args' or 'func' longer than necessary,

packages/exo/src/exo-makers.js

@@ -1,18 +1,14 @@
-/* global globalThis */
 /// <reference types="ses"/>
-import { makeEnvironmentCaptor } from '@endo/env-options';
+import { environmentOptionsListHas } from '@endo/env-options';
 import { objectMap } from '@endo/patterns';
 
 import { defendPrototype, defendPrototypeKit } from './exo-tools.js';
 
 const { Fail, quote: q } = assert;
 const { create, seal, freeze, defineProperty, values } = Object;
 
-const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis);
-const DEBUG = getEnvironmentOption('DEBUG', '');
-
 // Turn on to give each exo instance its own toStringTag value.
-const LABEL_INSTANCES = DEBUG.split(',').includes('label-instances');
+const LABEL_INSTANCES = environmentOptionsListHas('DEBUG', 'label-instances');
 
 /**
  * @template {{}} T

packages/ses-ava/test/test-env-options.js

@@ -37,3 +37,30 @@ test('test env options present', t => {
     message: 'Environment option default setting ["none"] must be a string.',
   });
 });
+
+test('test env options list', t => {
+  const c3 = new Compartment({
+    process: {
+      env: {
+        DEBUG: 'aa:bb,cc',
+      },
+    },
+  });
+  const {
+    getEnvironmentOption,
+    getEnvironmentOptionsList,
+    environmentOptionsListHas,
+    getCapturedEnvironmentOptionNames,
+  } = makeEnvironmentCaptor(c3.globalThis, true);
+
+  t.is(getEnvironmentOption('DEBUG', ''), 'aa:bb,cc');
+  t.deepEqual(getEnvironmentOption('DEBUG', '').split(':'), ['aa', 'bb,cc']);
+  t.deepEqual(getEnvironmentOptionsList('DEBUG'), ['aa:bb', 'cc']);
+  t.deepEqual(getEnvironmentOptionsList('FOO'), []);
+  t.is(environmentOptionsListHas('DEBUG', 'aa:bb'), true);
+  t.is(environmentOptionsListHas('DEBUG', 'aa'), false);
+  t.is(environmentOptionsListHas('FOO', 'aa'), false);
+
+  // Empty because of the `dropNames` `true` argument.
+  t.deepEqual(getCapturedEnvironmentOptionNames(), []);
+});

packages/ses/src/lockdown.js

@@ -14,7 +14,7 @@
 
 // @ts-check
 
-import { makeEnvironmentCaptor } from '@endo/env-options';
+import { getEnvironmentOption as getenv } from '@endo/env-options';
 import {
   FERAL_FUNCTION,
   FERAL_EVAL,
@@ -156,8 +156,6 @@ export const repairIntrinsics = (options = {}) => {
   // [`stackFiltering` options](https://github.com/Agoric/SES-shim/blob/master/packages/ses/docs/lockdown.md#stackfiltering-options)
   // for an explanation.
 
-  const { getEnvironmentOption: getenv } = makeEnvironmentCaptor(globalThis);
-
   const {
     errorTaming = getenv('LOCKDOWN_ERROR_TAMING', 'safe'),
     errorTrapping = /** @type {"platform" | "none" | "report" | "abort" | "exit" | undefined} */ (