Merge pull request #8505 from Anshumancanrock/fix-undefined-bug

Fix Show actual type in strands hook error messages

Author: davepagurek

src/strands/strands_api.js

@@ -485,7 +485,12 @@ function enforceReturnTypeMatch(strandsContext, expectedType, returned, hookName
   }
   if (receivedType.dimension !== expectedType.dimension) {
     if (receivedType.dimension !== 1) {
-      FES.userError('type error', `You have returned a vector with ${receivedType.dimension} components in ${hookName} when a ${expectedType.baseType + expectedType.dimension} was expected!`);
+      const receivedTypeDisplay = receivedType.baseType + (receivedType.dimension > 1 ? receivedType.dimension : '');
+      const expectedTypeDisplay = expectedType.baseType + expectedType.dimension;
+      FES.userError('type error',
+        `You have returned a ${receivedTypeDisplay} in ${hookName} when a ${expectedTypeDisplay} was expected!\n\n` +
+        `Make sure your hook returns the correct type.`
+      );
     }
     else {
       const result = build.primitiveConstructorNode(strandsContext, expectedType, returned);
@@ -576,7 +581,24 @@ export function createShaderHooksFunctions(strandsContext, fn, shader) {
           if (retNode instanceof StrandsNode) {
             const returnedNode = getNodeDataFromID(strandsContext.dag, retNode.id);
             if (returnedNode.baseType !== expectedStructType.typeName) {
-              FES.userError("type error", `You have returned a ${retNode.baseType} from ${hookType.name} when a ${expectedStructType.typeName} was expected.`);
+              const receivedTypeName = returnedNode.baseType || 'undefined';
+              const receivedDim = dag.dimensions[retNode.id];
+              const receivedTypeDisplay = receivedDim > 1 ?
+                `${receivedTypeName}${receivedDim}` : receivedTypeName;
+
+              const expectedProps = expectedStructType.properties
+                .map(p => p.name).join(', ');
+              FES.userError('type error',
+                `You have returned a ${receivedTypeDisplay} from ${hookType.name} when a ${expectedStructType.typeName} was expected.\n\n` +
+                `The ${expectedStructType.typeName} struct has these properties: { ${expectedProps} }\n\n` +
+                `Instead of returning a different type, you should modify and return the ${expectedStructType.typeName} struct that was passed to your hook.\n\n` +
+                `For example:\n` +
+                `${hookType.name}((inputs) => {\n` +
+                `  // Modify properties of inputs\n` +
+                `  inputs.someProperty = ...;\n` +
+                `  return inputs; // Return the modified struct\n` +
+                `})`
+              );
             }
             const newDeps = returnedNode.dependsOn.slice();
             for (let i = 0; i < expectedStructType.properties.length; i++) {
@@ -595,10 +617,14 @@ export function createShaderHooksFunctions(strandsContext, fn, shader) {
               const propName = expectedProp.name;
               const receivedValue = retNode[propName];
               if (receivedValue === undefined) {
-                FES.userError('type error', `You've returned an incomplete struct from ${hookType.name}.\n` +
-                  `Expected: { ${expectedReturnType.properties.map(p => p.name).join(', ')} }\n` +
-                  `Received: { ${Object.keys(retNode).join(', ')} }\n` +
-                  `All of the properties are required!`);
+                const expectedProps = expectedReturnType.properties.map(p => p.name).join(', ');
+                const receivedProps = Object.keys(retNode).join(', ');
+                FES.userError('type error',
+                  `You've returned an incomplete ${expectedStructType.typeName} struct from ${hookType.name}.\n\n` +
+                  `Expected properties: { ${expectedProps} }\n` +
+                  `Received properties: { ${receivedProps} }\n\n` +
+                  `All properties are required! Make sure to include all properties in the returned struct.`
+                );
               }
               const expectedTypeInfo = expectedProp.dataType;
               const returnedPropID = enforceReturnTypeMatch(strandsContext, expectedTypeInfo, receivedValue, hookType.name);

test/unit/webgl/p5.Shader.js

@@ -1,4 +1,16 @@
 import p5 from '../../../src/app.js';
+import { vi } from 'vitest';
+
+const mockUserError = vi.fn();
+vi.mock('../../../src/strands/strands_FES', () => ({
+  userError: (...args) => {
+    mockUserError(...args);
+    const prefixedMessage = `[p5.strands ${args[0]}]: ${args[1]}`;
+    throw new Error(prefixedMessage);
+  },
+  internalError: (msg) => { throw new Error(`[p5.strands internal error]: ${msg}`); }
+}));
+
 suite('p5.Shader', function() {
   var myp5;
   beforeAll(function() {
@@ -1942,4 +1954,60 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca
       assert.approximately(pixelColor[2], 0, 5);
     });
   });
+
+  suite('p5.strands error messages', () => {
+    afterEach(() => {
+      mockUserError.mockClear();
+    });
+
+    test('wrong type in struct hook shows actual type and expected properties', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(() => {
+          myp5.getWorldInputs(() => [1, 2, 3, 4]); // vec4 instead of Vertex struct
+        }, { myp5 });
+      } catch (e) { /* expected */ }
+
+      assert.isAbove(mockUserError.mock.calls.length, 0, 'FES.userError should have been called');
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.notInclude(errMsg, 'a undefined'); //
+      assert.include(errMsg, 'float4');
+      assert.include(errMsg, 'getWorldInputs');
+      assert.include(errMsg, 'Vertex');
+      assert.include(errMsg, 'properties');
+    });
+
+    test('vector dimension mismatch shows actual and expected types', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(() => {
+          myp5.getFinalColor((c) => [c.r, c.g, c.b]); // vec3 instead of vec4
+        }, { myp5 });
+      } catch (e) { /* expected */ }
+
+      assert.isAbove(mockUserError.mock.calls.length, 0, 'FES.userError should have been called');
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, 'float3');
+      assert.include(errMsg, 'float4');
+    });
+
+    test('incomplete struct shows expected vs received properties', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(() => {
+          myp5.getWorldInputs((inputs) => {
+            return { position: inputs.position };
+          });
+        }, { myp5 });
+      } catch (e) { /* expected */ }
+
+      assert.isAbove(mockUserError.mock.calls.length, 0, 'FES.userError should have been called');
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, 'Expected properties');
+      assert.include(errMsg, 'Received properties');
+    });
+  });
 });