NME: Bubble up build errors (#16731)

Try to catch what-would-be build errors via state-- so they can
eventually be bubbled up to the console UI via `onBuildErrorObservable`
notifications-- without throwing and breaking the build process.

Author: alexchuber

packages/dev/core/src/Materials/Node/Blocks/Dual/smartFilterTextureBlock.ts

@@ -8,7 +8,6 @@ import { InputBlock } from "../Input/inputBlock";
 import type { NodeMaterialBlock } from "../../nodeMaterialBlock";
 import type { NodeMaterial } from "../../nodeMaterial";
 import { ScreenSizeBlock } from "../Fragment/screenSizeBlock";
-import { Logger } from "core/Misc/logger";
 import { ShaderLanguage } from "core/Materials/shaderLanguage";
 import { editableInPropertyPage, PropertyTypeForEdition } from "core/Decorators/nodeDecorator";
 import type { Scene } from "core/scene";
@@ -55,11 +54,11 @@ export class SmartFilterTextureBlock extends CurrentScreenBlock {
         this._samplerName = state._getFreeVariableName(this.name);
 
         if (state.sharedData.nodeMaterial.mode !== NodeMaterialModes.SFE) {
-            Logger.Error("SmartFilterTextureBlock: Should not be used outside of SFE mode.");
+            state.sharedData.raiseBuildError("SmartFilterTextureBlock should not be used outside of SFE mode.");
         }
 
         if (state.sharedData.nodeMaterial.shaderLanguage !== ShaderLanguage.GLSL) {
-            Logger.Error("SmartFilterTextureBlock: WebGPU is not supported by SFE mode.");
+            state.sharedData.raiseBuildError("WebGPU is not supported by SmartFilterTextureBlock.");
         }
 
         // Tell FragmentOutputBlock ahead of time to store the final color in a temp variable
@@ -109,7 +108,7 @@ export class SmartFilterTextureBlock extends CurrentScreenBlock {
         // NOTE: In the future, when we move to vertex shaders, update this to check for the nearest vec2 varying output.
         const screenUv = state.sharedData.nodeMaterial.getInputBlockByPredicate((b) => b.isAttribute && b.name === "postprocess_uv");
         if (!screenUv || !screenUv.isAnAncestorOf(this)) {
-            Logger.Error("SmartFilterTextureBlock: 'postprocess_uv' attribute from ScreenUVBlock is required.");
+            state.sharedData.raiseBuildError("SmartFilterTextureBlock: 'postprocess_uv' attribute from ScreenUVBlock is required.");
             return "";
         }
         return screenUv.associatedVariableName;

packages/dev/core/src/Materials/Node/Blocks/Fragment/fragCoordBlock.ts

@@ -102,8 +102,8 @@ export class FragCoordBlock extends NodeMaterialBlock {
         super._buildBlock(state);
 
         if (state.target === NodeMaterialBlockTargets.Vertex) {
-            // eslint-disable-next-line no-throw-literal
-            throw "FragCoordBlock must only be used in a fragment shader";
+            state.sharedData.raiseBuildError("FragCoordBlock must only be used in a fragment shader");
+            return this;
         }
 
         state.compilationString += this.writeOutputs(state);

packages/dev/core/src/Materials/Node/Blocks/Fragment/frontFacingBlock.ts

@@ -38,8 +38,8 @@ export class FrontFacingBlock extends NodeMaterialBlock {
         super._buildBlock(state);
 
         if (state.target === NodeMaterialBlockTargets.Vertex) {
-            // eslint-disable-next-line no-throw-literal
-            throw "FrontFacingBlock must only be used in a fragment shader";
+            state.sharedData.raiseBuildError("FrontFacingBlock must only be used in a fragment shader");
+            return this;
         }
 
         const output = this._outputs[0];

packages/dev/core/src/Materials/Node/Blocks/Fragment/heightToNormalBlock.ts

@@ -6,7 +6,6 @@ import { NodeMaterialBlockTargets } from "../../Enums/nodeMaterialBlockTargets";
 import { RegisterClass } from "../../../../Misc/typeStore";
 import { editableInPropertyPage, PropertyTypeForEdition } from "../../../../Decorators/nodeDecorator";
 import type { Scene } from "../../../../scene";
-import { Logger } from "../../../../Misc/logger";
 import { ShaderLanguage } from "../../../../Materials/shaderLanguage";
 
 /**
@@ -109,7 +108,7 @@ export class HeightToNormalBlock extends NodeMaterialBlock {
         const fPrefix = state.fSuffix;
 
         if (!this.generateInWorldSpace && !this.worldTangent.isConnected) {
-            Logger.Error(`You must connect the 'worldTangent' input of the ${this.name} block!`);
+            state.sharedData.raiseBuildError(`You must connect the 'worldTangent' input of the ${this.name} block!`);
         }
 
         const startCode = this.generateInWorldSpace

packages/dev/core/src/Materials/Node/Blocks/Fragment/screenSizeBlock.ts

@@ -88,8 +88,8 @@ export class ScreenSizeBlock extends NodeMaterialBlock {
         this._scene = state.sharedData.scene;
 
         if (state.target === NodeMaterialBlockTargets.Vertex) {
-            // eslint-disable-next-line no-throw-literal
-            throw "ScreenSizeBlock must only be used in a fragment shader";
+            state.sharedData.raiseBuildError("ScreenSizeBlock must only be used in a fragment shader");
+            return this;
         }
 
         state.sharedData.bindableBlocks.push(this);

packages/dev/core/src/Materials/Node/Blocks/PBR/anisotropyBlock.ts

@@ -9,7 +9,6 @@ import { NodeMaterialConnectionPointCustomObject } from "../../nodeMaterialConne
 import { TBNBlock } from "../Fragment/TBNBlock";
 import type { Mesh } from "../../../../Meshes/mesh";
 import type { Effect } from "../../../effect";
-import { Logger } from "core/Misc/logger";
 import type { NodeMaterialBuildState } from "../../nodeMaterialBuildState";
 import { ShaderLanguage } from "core/Materials/shaderLanguage";
 
@@ -141,7 +140,7 @@ export class AnisotropyBlock extends NodeMaterialBlock {
             // we must set the uv input as optional because we may not end up in this method (in case a PerturbNormal block is linked to the PBR material)
             // in which case uv is not required. But if we do come here, we do need the uv, so we have to raise an error but not with throw, else
             // it will stop the building of the node material and will lead to errors in the editor!
-            Logger.Error("You must connect the 'uv' input of the Anisotropy block!");
+            state.sharedData.raiseBuildError(`You must connect the 'uv' input of the ${this.name} block!`);
         }
 
         state._emitExtension("derivatives", "#extension GL_OES_standard_derivatives : enable");

packages/dev/core/src/Materials/Node/Blocks/PBR/pbrMetallicRoughnessBlock.ts

@@ -1492,7 +1492,7 @@ export class PBRMetallicRoughnessBlock extends NodeMaterialBlock {
                         state.compilationString += `#endif\n`;
                     }
                 } else {
-                    Logger.Error(`There's no remapping for the ${output.name} end point! No code generated`);
+                    state.sharedData.raiseBuildError(`There's no remapping for the ${output.name} end point! No code generated`);
                 }
             }
         }

packages/dev/core/src/Materials/Node/nodeMaterial.ts

@@ -784,8 +784,8 @@ export class NodeMaterial extends PushMaterial {
 
                 for (const other of this.attachedBlocks) {
                     if (other.getClassName() === className) {
-                        // eslint-disable-next-line no-throw-literal
-                        throw `Cannot have multiple blocks of type ${className} in the same NodeMaterial`;
+                        this._sharedData.raiseBuildError(`Cannot have multiple blocks of type ${className} in the same NodeMaterial`);
+                        return;
                     }
                 }
             }
@@ -915,13 +915,15 @@ export class NodeMaterial extends PushMaterial {
         const allowEmptyVertexProgram = this._mode === NodeMaterialModes.Particle || this._mode === NodeMaterialModes.SFE;
 
         if (this._vertexOutputNodes.length === 0 && !allowEmptyVertexProgram) {
-            // eslint-disable-next-line no-throw-literal
-            throw "You must define at least one vertexOutputNode";
+            this.onBuildErrorObservable.notifyObservers("You must define at least one vertexOutputNode");
+            this._buildIsInProgress = false;
+            return;
         }
 
         if (this._fragmentOutputNodes.length === 0) {
-            // eslint-disable-next-line no-throw-literal
-            throw "You must define at least one fragmentOutputNode";
+            this.onBuildErrorObservable.notifyObservers("You must define at least one fragmentOutputNode");
+            this._buildIsInProgress = false;
+            return;
         }
 
         // Compilation state
@@ -1021,20 +1023,20 @@ export class NodeMaterial extends PushMaterial {
             this._buildId = NodeMaterial._BuildIdGenerator++;
         }
 
-        // Errors
-        const noError = this._sharedData.emitErrors(this.onBuildErrorObservable);
-
         if (verbose) {
             Logger.Log("Vertex shader:");
             Logger.Log(this._vertexCompilationState.compilationString);
             Logger.Log("Fragment shader:");
             Logger.Log(this._fragmentCompilationState.compilationString);
         }
 
+        // Errors
+        const noError = this._sharedData.emitErrors();
+
         this._buildIsInProgress = false;
-        this._buildWasSuccessful = true;
         if (noError) {
             this.onBuildObservable.notifyObservers(this);
+            this._buildWasSuccessful = true;
         }
 
         // Wipe defines

packages/dev/core/src/Materials/Node/nodeMaterialBuildState.ts

@@ -9,6 +9,7 @@ import type { NodeMaterialBlock } from "./nodeMaterialBlock";
 import { Process } from "core/Engines/Processors/shaderProcessor";
 import type { _IProcessingOptions } from "core/Engines/Processors/shaderProcessingOptions";
 import { WebGLShaderProcessor } from "core/Engines/WebGL/webGLShaderProcessors";
+import { Logger } from "core/Misc/logger";
 
 /**
  * Class used to store node based material build state
@@ -112,7 +113,8 @@ export class NodeMaterialBuildState {
      */
     public async getProcessedShaderAsync(defines: string): Promise<string> {
         if (!this._builtCompilationString) {
-            throw new Error("Shader not built yet.");
+            Logger.Error("getProcessedShaderAsync: Shader not built yet.");
+            return "";
         }
 
         const engine = this.sharedData.nodeMaterial.getScene().getEngine();

packages/dev/core/src/Materials/Node/nodeMaterialBuildStateSharedData.ts

@@ -5,7 +5,6 @@ import type { Scene } from "../../scene";
 import type { Immutable, Nullable } from "../../types";
 import type { NodeMaterial, NodeMaterialTextureBlocks } from "./nodeMaterial";
 import { Logger } from "core/Misc/logger";
-import type { Observable } from "core/Misc/observable";
 
 /**
  * Class used to store shared data between 2 NodeMaterialBuildState
@@ -143,6 +142,7 @@ export class NodeMaterialBuildStateSharedData {
         emitVertex: false,
         emitFragment: false,
         notConnectedNonOptionalInputs: new Array<NodeMaterialConnectionPoint>(),
+        customErrors: new Array<string>(),
     };
 
     /**
@@ -187,12 +187,21 @@ export class NodeMaterialBuildStateSharedData {
         this.defineNames["MAINUV7"] = 0;
     }
 
+    /**
+     * Push a new error to the build state, avoiding exceptions that can break the build process
+     * @param message defines the error message to push
+     */
+    public raiseBuildError(message: string) {
+        if (this.checks.customErrors.indexOf(message) !== -1) {
+            this.checks.customErrors.push(message);
+        }
+    }
+
     /**
      * Emits console errors and exceptions if there is a failing check
-     * @param errorObservable defines an Observable to send the error message
      * @returns true if all checks pass
      */
-    public emitErrors(errorObservable: Nullable<Observable<string>> = null) {
+    public emitErrors() {
         let errorMessage = "";
 
         if (!this.checks.emitVertex && !this.allowEmptyVertexProgram) {
@@ -206,13 +215,14 @@ export class NodeMaterialBuildStateSharedData {
                 notConnectedInput.ownerBlock.name
             }[${notConnectedInput.ownerBlock.getClassName()}] is not connected and is not optional.\n`;
         }
+        for (const customError of this.checks.customErrors) {
+            errorMessage += customError + "\n";
+        }
 
         if (errorMessage) {
-            if (errorObservable) {
-                errorObservable.notifyObservers(errorMessage);
-            }
-            Logger.Error("Build of NodeMaterial failed:\n" + errorMessage);
-
+            errorMessage = "Node material build failed: \n" + errorMessage;
+            Logger.Error(errorMessage);
+            this.nodeMaterial.onBuildErrorObservable.notifyObservers(errorMessage);
             return false;
         }