getConfig: handle instances of useage where should throw error in prod but not in test

Author: clairep94

client/modules/IDE/actions/project.js

@@ -15,9 +15,11 @@ import {
 } from './ide';
 import { clearState, saveState } from '../../../persistState';
 
-const ROOT_URL = getConfig('API_URL', { nullishString: true });
-const S3_BUCKET_URL_BASE = getConfig('S3_BUCKET_URL_BASE');
-const S3_BUCKET = getConfig('S3_BUCKET');
+const ROOT_URL = getConfig('API_URL', { failOnNotFound: true });
+const S3_BUCKET_URL_BASE = getConfig('S3_BUCKET_URL_BASE', {
+  failOnNotFound: true
+});
+const S3_BUCKET = getConfig('S3_BUCKET', { failOnNotFound: true });
 
 export function setProject(project) {
   return {

client/modules/IDE/actions/uploader.js

@@ -1,15 +1,20 @@
 import { TEXT_FILE_REGEX } from '../../../../server/utils/fileUtils';
 import { apiClient } from '../../../utils/apiClient';
-import { getConfig } from '../../../utils/getConfig';
+import { getConfig, isTestEnvironment } from '../../../utils/getConfig';
 import { handleCreateFile } from './files';
 
+const s3BucketUrlBase = getConfig('S3_BUCKET_URL_BASE');
+const awsRegion = getConfig('AWS_REGION');
+const s3Bucket = getConfig('S3_BUCKET');
+
+if (!isTestEnvironment && !s3BucketUrlBase && !(awsRegion && s3Bucket)) {
+  throw new Error(`S3 bucket address not configured. 
+    Configure either S3_BUCKET_URL_BASE or both AWS_REGION & S3_BUCKET in env vars`);
+}
+
 export const s3BucketHttps =
-  getConfig('S3_BUCKET_URL_BASE') ||
-  `https://s3-${getConfig('AWS_REGION', {
-    nullishString: true
-  })}.amazonaws.com/${getConfig('S3_BUCKET', {
-    nullishString: true
-  })}/`;
+  s3BucketUrlBase || `https://s3-${awsRegion}.amazonaws.com/${s3Bucket}/`;
+
 const MAX_LOCAL_FILE_SIZE = 80000; // bytes, aka 80 KB
 
 function isS3Upload(file) {

client/modules/IDE/components/PreviewFrame.jsx

@@ -13,7 +13,7 @@ const Frame = styled.iframe`
 
 function PreviewFrame({ fullView, isOverlayVisible }) {
   const iframe = useRef();
-  const previewUrl = getConfig('PREVIEW_URL');
+  const previewUrl = getConfig('PREVIEW_URL', { failOnNotFound: true });
   useEffect(() => {
     const unsubscribe = registerFrame(iframe.current.contentWindow, previewUrl);
     return () => {

client/modules/IDE/components/SketchListRowBase.jsx

@@ -11,7 +11,7 @@ import MenuItem from '../../../components/Dropdown/MenuItem';
 import { formatDateToString } from '../../../utils/formatDate';
 import { getConfig } from '../../../utils/getConfig';
 
-const ROOT_URL = getConfig('API_URL', { nullishString: true });
+const ROOT_URL = getConfig('API_URL', { failOnNotFound: true });
 
 const formatDateCell = (date, mobile = false) =>
   formatDateToString(date, { showTime: !mobile });

client/modules/Preview/EmbedFrame.jsx

@@ -232,9 +232,9 @@ p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);`
   }
 
   const previewScripts = sketchDoc.createElement('script');
-  previewScripts.src = `${window.location.origin}${
-    (getConfig('PREVIEW_SCRIPTS_URL'), { nullishString: true })
-  }`;
+  previewScripts.src = `${
+    window.location.origin
+  }${getConfig('PREVIEW_SCRIPTS_URL', { nullishString: true })}`;
   previewScripts.setAttribute('crossorigin', '');
   sketchDoc.head.appendChild(previewScripts);
 
@@ -245,9 +245,9 @@ p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);`
     window.offs = ${JSON.stringify(scriptOffs)};
     window.objectUrls = ${JSON.stringify(objectUrls)};
     window.objectPaths = ${JSON.stringify(objectPaths)};
-    window.editorOrigin = '${
-      (getConfig('EDITOR_URL'), { nullishString: true })
-    }';
+    window.editorOrigin = '${getConfig('EDITOR_URL', {
+      failOnNotFound: true
+    })}';
   `;
   addLoopProtect(sketchDoc);
   sketchDoc.head.prepend(consoleErrorsScript);

client/modules/Preview/previewIndex.jsx

@@ -9,7 +9,7 @@ import {
 } from '../../utils/dispatcher';
 import { filesReducer, setFiles } from './filesReducer';
 import EmbedFrame from './EmbedFrame';
-import { getConfig } from '../../utils/getConfig';
+import { getConfig, isTestEnvironment } from '../../utils/getConfig';
 import { initialState } from '../IDE/reducers/files';
 
 const GlobalStyle = createGlobalStyle`
@@ -24,7 +24,10 @@ const App = () => {
   const [basePath, setBasePath] = useState('');
   const [textOutput, setTextOutput] = useState(false);
   const [gridOutput, setGridOutput] = useState(false);
-  registerFrame(window.parent, getConfig('EDITOR_URL'));
+  registerFrame(
+    window.parent,
+    getConfig('EDITOR_URL', { failOnNotFound: true })
+  );
 
   function handleMessageEvent(message) {
     const { type, payload } = message;

client/modules/User/components/CookieConsent.jsx

@@ -77,6 +77,8 @@ const CookieConsentButtons = styled.div`
   }
 `;
 
+const GOOGLE_ANALYTICS_ID = getConfig('GA_MEASUREMENT_ID');
+
 function CookieConsent({ hide }) {
   const user = useSelector((state) => state.user);
   const [cookieConsent, setBrowserCookieConsent] = useState('none');
@@ -133,15 +135,15 @@ function CookieConsent({ hide }) {
       initializeCookieConsent();
     }
 
-    if (getConfig('GA_MEASUREMENT_ID')) {
+    if (GOOGLE_ANALYTICS_ID) {
       if (p5CookieConsent === 'essential') {
-        ReactGA.initialize(getConfig('GA_MEASUREMENT_ID'), {
+        ReactGA.initialize(GOOGLE_ANALYTICS_ID, {
           gaOptions: {
             storage: 'none'
           }
         });
       } else {
-        ReactGA.initialize(getConfig('GA_MEASUREMENT_ID'));
+        ReactGA.initialize(GOOGLE_ANALYTICS_ID);
       }
       ReactGA.pageview(window.location.pathname + window.location.search);
     }

client/utils/apiClient.ts

@@ -1,7 +1,7 @@
 import axios, { AxiosInstance } from 'axios';
 import { getConfig } from './getConfig';
 
-const ROOT_URL = getConfig('API_URL', { nullishString: true });
+const ROOT_URL = getConfig('API_URL', { failOnNotFound: true });
 
 /**
  * Configures an Axios instance with the correct API URL

client/utils/getConfig.test.ts

@@ -33,8 +33,12 @@ describe('utils/getConfig()', () => {
     });
 
     it('throws an error if failOnNotFound is true', () => {
+      process.env.NODE_ENV = 'not_test';
       expect(() =>
-        getConfig('CONFIG_TEST_KEY_NAME', { failOnNotFound: true })
+        getConfig('CONFIG_TEST_KEY_NAME', {
+          failOnNotFound: true,
+          failInTestEnv: true
+        })
       ).toThrow();
     });
 
@@ -62,7 +66,10 @@ describe('utils/getConfig()', () => {
 
     it('throws an error if failOnNotFound is true', () => {
       expect(() =>
-        getConfig('CONFIG_TEST_KEY_NAME', { failOnNotFound: true })
+        getConfig('CONFIG_TEST_KEY_NAME', {
+          failOnNotFound: true,
+          failInTestEnv: true
+        })
       ).toThrow();
     });
 

client/utils/getConfig.ts

@@ -13,14 +13,16 @@ interface GetConfigOptions {
   warn?: boolean;
   nullishString?: boolean;
   failOnNotFound?: boolean;
+  failInTestEnv?: boolean; // this is only to test getConfig and should never be set to override defaults
 }
 
-const isTestEnvironment = getEnvVar('NODE_ENV') === 'test';
+export const isTestEnvironment = getEnvVar('NODE_ENV') === 'test';
 
 const defaultGetConfigOptions: GetConfigOptions = {
   warn: !isTestEnvironment,
   nullishString: false,
-  failOnNotFound: false
+  failOnNotFound: false,
+  failInTestEnv: false
 };
 
 /**
@@ -43,7 +45,7 @@ export function getConfig(
   }
 
   // override default options with param options
-  const { warn, nullishString, failOnNotFound } = {
+  const { warn, nullishString, failOnNotFound, failInTestEnv } = {
     ...defaultGetConfigOptions,
     ...options
   };
@@ -56,7 +58,10 @@ export function getConfig(
     const notFoundMessage = `getConfig("${key}") returned null or undefined`;
 
     // error, warn or continue if no value found:
-    if (failOnNotFound) {
+    if (
+      (failOnNotFound && !isTestEnvironment) ||
+      (failOnNotFound && isTestEnvironment && failInTestEnv) // this is just to enable us to test getConfig's error throwing
+    ) {
       throw new Error(notFoundMessage);
     }
     if (warn) {