From eef77058ab919e2a0a486c8fe2b36e4d3adb679d Mon Sep 17 00:00:00 2001
From: Peter Bengtsson <mail@peterbe.com>
Date: Mon, 28 Feb 2022 15:40:06 -0500
Subject: [PATCH] throw if no value not found in indented_data_reference
 (#25711)

* throw if no value not found in indented_data_reference

* better comment

* deliberate writer-error saple

* all strings are truthy'ish

* undo

* fix tests
---
 lib/liquid-tags/indented-data-reference.js | 32 ++++++++++++++++++++--
 tests/content/site-data.js                 | 17 +++++++-----
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/lib/liquid-tags/indented-data-reference.js b/lib/liquid-tags/indented-data-reference.js
index babc582326..bd4d002e74 100644
--- a/lib/liquid-tags/indented-data-reference.js
+++ b/lib/liquid-tags/indented-data-reference.js
@@ -1,5 +1,15 @@
 import assert from 'assert'
 
+// If 'THROW_ON_EMPTY' is set and it's value is '0' or 'false' it becomes
+// false. Or true if it's 'true' or '1'.
+const THROW_ON_EMPTY = Boolean(
+  process.env.THROW_ON_EMPTY
+    ? JSON.parse(process.env.THROW_ON_EMPTY)
+    : JSON.parse(process.env.CI || process.env.NODE_ENV !== 'production')
+)
+
+class IndentedDataReferenceError extends Error {}
+
 // This class supports a tag that expects two parameters, a data reference and `spaces=NUMBER`:
 //
 // {% indented_data_reference foo.bar spaces=NUMBER %}
@@ -33,9 +43,25 @@ export default {
     // Get the referenced value from the context
     const value = await this.liquid.evalValue(`site.data.${dataReference}`, scope)
 
-    // If nothing is found in the context, exit with nothing; this may
-    // feel weird and that we should throw an error, but this is "The Liquid Way TM"
-    if (!value) return
+    // If value is falsy it can be because we completely failed to look
+    // it up. But it can also be literally an empty string.
+    // For example, the reusable could be entirely something like this:
+    //
+    //   {% if some condition %}The meat{% endif %}
+    //
+    // Then it's working as expected. But if the reference is wrong, e.g.
+    //
+    //   {% indented_data_reference reusables.foo.tyypu spaces=3 %}
+    //
+    // Or if the file simple doesn't exist, you get an undefined for the value.
+    if (typeof value !== 'string' && !value) {
+      const message = `Can't find the key 'site.data.${dataReference}' in the scope.`
+      if (THROW_ON_EMPTY) {
+        throw new IndentedDataReferenceError(message)
+      }
+      console.warn(message)
+      return
+    }
 
     // add spaces to each line
     const renderedReferenceWithIndent = value.replace(/^/gm, ' '.repeat(numSpaces))
diff --git a/tests/content/site-data.js b/tests/content/site-data.js
index 469885e456..2bf594f529 100644
--- a/tests/content/site-data.js
+++ b/tests/content/site-data.js
@@ -3,6 +3,7 @@ import path from 'path'
 import fs from 'fs'
 import { get, isPlainObject, has } from 'lodash-es'
 import flat from 'flat'
+import { ParseError } from 'liquidjs'
 import loadSiteData from '../../lib/site-data.js'
 import patterns from '../../lib/patterns.js'
 import { liquid } from '../../lib/render-content/index.js'
@@ -63,20 +64,22 @@ describe('siteData module (English)', () => {
     }
   })
 
-  test('all Liquid templating is valid', async () => {
+  test('all Liquid tags are valid', async () => {
     const dataMap = flat(data)
     for (const key in dataMap) {
       const value = dataMap[key]
       if (!patterns.hasLiquid.test(value)) continue
-      let message = `${key} contains a malformed Liquid expression`
-      let result = null
       try {
-        result = await liquid.parseAndRender(value)
+        await liquid.parseAndRender(value)
       } catch (err) {
-        console.trace(err)
-        message += `: ${err.message}`
+        if (err instanceof ParseError) {
+          console.warn('value that failed to parse:', value)
+          throw new Error(`Unable to parse with Liquid: ${err.message}`)
+        }
+        // Note, the parseAndRender() might throw other errors. For
+        // example errors about the the data. But at least it
+        // managed to get paste the Liquid parsing phase.
       }
-      expect(typeof result, message).toBe('string')
     }
   })