fix errors in dynamic component being silent

fixes #314

Author: lovasoa

CHANGELOG.md

@@ -11,6 +11,7 @@
    ```
  - Casting values from one type to another using the `::` operator is only supported by PostgreSQL. SQLPage versions before 0.20.5 would silently convert all casts to the `CAST(... AS ...)` syntax, which is supported by all databases. Since 0.20.5, SQLPage started to respect the original `::` syntax, and pass it as-is to the database. This broke existing SQLPage websites that used the `::` syntax with databases other than PostgreSQL. For backward compatibility, this version of SQLPage re-establishes the previous behavior, converts `::` casts on non-PostgreSQL databases to the `CAST(... AS ...)` syntax, but will display a warning in the logs. 
     - In short, if you saw an error like `Error: unrecognized token ":"` after upgrading to 0.20.5, this version should fix it.
+ - The `dynamic` component now properly displays error messages when its properties are invalid. There used to be a bug where errors would be silently ignored, making it hard to debug invalid dynamic components.
 
 ## 0.20.5 (2024-05-07)
 

src/dynamic_component.rs

@@ -40,11 +40,19 @@ impl Iterator for DynamicComponentIterator {
 }
 
 fn expand_dynamic_stack(stack: &mut Vec<anyhow::Result<JsonValue>>) {
-    while let Some(Ok(mut next)) = stack.pop() {
-        if let Some(properties) = extract_dynamic_properties(&mut next) {
+    while let Some(mut next) = stack.pop() {
+        let dyn_props = next
+            .as_mut()
+            .ok()
+            .and_then(|mut v| extract_dynamic_properties(&mut v));
+        if let Some(properties) = dyn_props {
+            // if the properties contain new (nested) dynamic components, push them onto the stack
             stack.extend(dynamic_properties_to_vec(properties));
         } else {
-            stack.push(Ok(next));
+            // If the properties are not dynamic, push the row back onto the stack
+            stack.push(next);
+            // return at the first non-dynamic row
+            // we don't support non-dynamic rows after dynamic rows nested in the same array
             return;
         }
     }
@@ -79,12 +87,8 @@ fn dynamic_properties_to_result_vec(
     mut properties_obj: JsonValue,
 ) -> anyhow::Result<Vec<JsonValue>> {
     if let JsonValue::String(s) = properties_obj {
-        properties_obj = serde_json::from_str::<JsonValue>(&s).with_context(|| {
-            format!(
-                "Unable to parse the 'properties' property of the dynamic component as JSON.\n\
-                    Invalid json: {s}"
-            )
-        })?;
+        properties_obj = serde_json::from_str::<JsonValue>(&s)
+            .with_context(|| format!("Invalid json in dynamic component properties: {s}"))?;
     }
     match properties_obj {
         obj @ JsonValue::Object(_) => Ok(vec![obj]),

tests/sql_test_files/error_dynamic_component_expected_properties_of_type_array_or_object.sql

@@ -0,0 +1 @@
+select 'dynamic' as component; -- missing "properties" column

tests/sql_test_files/error_invalid_json_in_dynamic_component.sql

@@ -0,0 +1,2 @@
+select 'dynamic' as component,
+    '{ "this object": "has a forbidden comma" , }' as properties; -- this is invalid JSON, and should cause an error