fix: improve error handling/reporting and prevent premature exit in dev mode

packages/build/src/build/build.ts

@@ -73,16 +73,14 @@ export async function build(mode: BuildMode, options: BuildOptions): Promise<Res
   const overlayEnabled = mode === 'development'
   setOverlayFile(messagesPath, overlayEnabled)
 
-  // Initialize plugins
-  const plugins = userConfig.plugins || []
-  for (const plugin of plugins) {
-    if (plugin.init) {
-      await plugin.init(resolvedConfig)
-    }
-  }
-
   try {
+    // Initialize plugins
     const plugins = userConfig.plugins || []
+    for (const plugin of plugins) {
+      if (plugin.init) {
+        await plugin.init(resolvedConfig)
+      }
+    }
 
     if (mode === 'development') {
       // Enable dev mode (which will rebuild when watched files are changed).

packages/build/src/build/impl/build-once.ts

@@ -17,7 +17,6 @@ import type { Plugin } from '../../plugin/plugin'
 
 import { generateModel } from './gen-model'
 import { computeInputFilesHash } from './hash-files'
-import type { ModelSpec } from '../../_shared/model-spec'
 
 export interface BuildOnceOptions {
   forceModelGen?: boolean
@@ -47,57 +46,55 @@ export async function buildOnce(
   // Create the build context
   const stagedFiles = new StagedFiles(config.prepDir)
   const context = new BuildContext(config, stagedFiles, options.abortSignal)
+  const modelHashPath = joinPath(config.prepDir, 'model-hash.txt')
 
-  // Get the model spec from the config
-  let modelSpec: ModelSpec
+  // Note that the entire body of this function is wrapped in a try/catch.  Any
+  // errors that are thrown by plugin functions or the core `generateModel`
+  // function will be caught and handled as appropriate.
+  let succeeded = true
   try {
-    modelSpec = await userConfig.modelSpec(context)
+    // Get the model spec from the config
+    const modelSpec = await userConfig.modelSpec(context)
     if (modelSpec === undefined) {
       return err(new Error('The model spec must be defined'))
     }
-  } catch (e) {
-    return err(e)
-  }
 
-  // Run plugins that implement `preGenerate`
-  for (const plugin of plugins) {
-    if (plugin.preGenerate) {
-      plugin.preGenerate(context, modelSpec)
+    // Run plugins that implement `preGenerate`
+    for (const plugin of plugins) {
+      if (plugin.preGenerate) {
+        await plugin.preGenerate(context, modelSpec)
+      }
     }
-  }
 
-  // Write the spec file
-  const specJson = {
-    inputVarNames: modelSpec.inputs.map(input => input.varName),
-    outputVarNames: modelSpec.outputs.map(output => output.varName),
-    externalDatfiles: modelSpec.datFiles,
-    ...modelSpec.options
-  }
-  const specPath = joinPath(config.prepDir, 'spec.json')
-  await writeFile(specPath, JSON.stringify(specJson, null, 2))
+    // Write the spec file
+    const specJson = {
+      inputVarNames: modelSpec.inputs.map(input => input.varName),
+      outputVarNames: modelSpec.outputs.map(output => output.varName),
+      externalDatfiles: modelSpec.datFiles,
+      ...modelSpec.options
+    }
+    const specPath = joinPath(config.prepDir, 'spec.json')
+    await writeFile(specPath, JSON.stringify(specJson, null, 2))
 
-  // Read the hash from the last successful model build, if available
-  const modelHashPath = joinPath(config.prepDir, 'model-hash.txt')
-  let previousModelHash: string
-  if (existsSync(modelHashPath)) {
-    previousModelHash = readFileSync(modelHashPath, 'utf8')
-  } else {
-    previousModelHash = 'NONE'
-  }
+    // Read the hash from the last successful model build, if available
+    let previousModelHash: string
+    if (existsSync(modelHashPath)) {
+      previousModelHash = readFileSync(modelHashPath, 'utf8')
+    } else {
+      previousModelHash = 'NONE'
+    }
 
-  // The code gen and Wasm build steps are time consuming, so we avoid rebuilding
-  // it if the build input files are unchanged since the last successful build
-  const inputFilesHash = await computeInputFilesHash(config)
-  let needModelGen: boolean
-  if (options.forceModelGen === true) {
-    needModelGen = true
-  } else {
-    const hashMismatch = inputFilesHash !== previousModelHash
-    needModelGen = hashMismatch
-  }
+    // The code gen and Wasm build steps are time consuming, so we avoid rebuilding
+    // it if the build input files are unchanged since the last successful build
+    const inputFilesHash = await computeInputFilesHash(config)
+    let needModelGen: boolean
+    if (options.forceModelGen === true) {
+      needModelGen = true
+    } else {
+      const hashMismatch = inputFilesHash !== previousModelHash
+      needModelGen = hashMismatch
+    }
 
-  let succeeded = true
-  try {
     if (needModelGen) {
       // Generate the model
       await generateModel(context, plugins)

packages/build/src/build/impl/gen-model.ts

@@ -96,7 +96,14 @@ async function preprocessMdl(
   // Use SDE to preprocess the model to strip anything that's not needed to build it
   const command = sdeCmdPath
   const args = ['generate', '--preprocess', 'processed.mdl']
-  await context.spawnChild(prepDir, command, args)
+  const ppOutput = await context.spawnChild(prepDir, command, args, {
+    // The default error message from `spawnChild` is not very informative, so the
+    // following allows us to throw our own error
+    ignoreError: true
+  })
+  if (ppOutput.exitCode !== 0) {
+    throw new Error(`Failed to preprocess mdl file: 'sde generate' command failed (code=${ppOutput.exitCode})`)
+  }
 
   // Copy the processed file back to the prep directory
   await copyFile(joinPath(prepDir, 'build', 'processed.mdl'), joinPath(prepDir, 'processed.mdl'))
@@ -148,9 +155,9 @@ async function flattenMdls(
         log('error', `  ${line}`)
       }
     }
-    throw new Error(`Flatten command failed (code=${output.exitCode})`)
+    throw new Error(`Failed to flatten mdl files: 'sde flatten' command failed (code=${output.exitCode})`)
   } else if (output.exitCode !== 0) {
-    throw new Error(`Flatten command failed (code=${output.exitCode})`)
+    throw new Error(`Failed to flatten mdl files: 'sde flatten' command failed (code=${output.exitCode})`)
   }
 
   // Copy the processed file back to the prep directory
@@ -167,11 +174,17 @@ async function generateC(context: BuildContext, sdeDir: string, sdeCmdPath: stri
   // dimensions and variables (`--list`)
   const command = sdeCmdPath
   const gencArgs = ['generate', '--genc', '--list', '--spec', 'spec.json', 'processed']
-  await context.spawnChild(prepDir, command, gencArgs, {
+  const gencOutput = await context.spawnChild(prepDir, command, gencArgs, {
     // By default, ignore lines that start with "WARNING: Data for" since these are often harmless
     // TODO: Don't filter by default, but make it configurable
     // ignoredMessageFilter: 'WARNING: Data for'
+    // The default error message from `spawnChild` is not very informative, so the
+    // following allows us to throw our own error
+    ignoreError: true
   })
+  if (gencOutput.exitCode !== 0) {
+    throw new Error(`Failed to generate C code: 'sde generate' command failed (code=${gencOutput.exitCode})`)
+  }
 
   // Copy SDE's supporting C files into the build directory
   const buildDir = joinPath(prepDir, 'build')

packages/build/tests/_shared/submodel1.mdl

@@ -0,0 +1,20 @@
+{UTF-8}
+
+X = TIME ~~|
+
+Y = 0
+  ~ [-10,10,0.1]
+  ~
+  |
+
+Z = X + Y
+  ~
+  ~ This is used for the "flatten" test in `build-prod.spec.ts`. \
+    It intentionally has a different definition than the Z equation \
+    in `submodel2.mdl` to trigger a flatten error.
+  |
+
+INITIAL TIME = 2000 ~~|
+FINAL TIME = 2100 ~~|
+TIME STEP = 1 ~~|
+SAVEPER = TIME STEP ~~|

packages/build/tests/_shared/submodel2.mdl

@@ -0,0 +1,13 @@
+{UTF-8}
+
+Z = 10
+  ~
+  ~ This is used for the "flatten" test in `build-prod.spec.ts`. \
+    It intentionally has a different definition than the Z equation \
+    in `submodel1.mdl` to trigger a flatten error.
+  |
+
+INITIAL TIME = 2000 ~~|
+FINAL TIME = 2100 ~~|
+TIME STEP = 1 ~~|
+SAVEPER = TIME STEP ~~|

packages/build/tests/build/build-prod.spec.ts

@@ -129,4 +129,131 @@ describe('build in production mode', () => {
       'plugin 2: postBuild'
     ])
   })
+
+  describe('should fail if plugin throws error', () => {
+    async function buildSample(plugin: Plugin): Promise<string> {
+      const userConfig: UserConfig = {
+        rootDir: resolvePath(__dirname, '..'),
+        prepDir: resolvePath(__dirname, 'sde-prep'),
+        modelFiles: [resolvePath(__dirname, '..', '_shared', 'sample.mdl')],
+        modelSpec: async () => {
+          return modelSpec
+        },
+        plugins: [plugin]
+      }
+
+      const result = await build('production', buildOptions(userConfig))
+      if (result.isOk()) {
+        throw new Error('Expected err result but got: ' + result.value)
+      }
+
+      return result.error.message
+    }
+
+    async function verify(pluginFunc: keyof Plugin): Promise<void> {
+      const plugin = {} as Plugin
+      plugin[pluginFunc] = async () => {
+        throw new Error(`${pluginFunc} error`)
+      }
+      const msg = await buildSample(plugin)
+      expect(msg).toBe(`${pluginFunc} error`)
+    }
+
+    it('in init', async () => verify('init'))
+    it('in preGenerate', async () => verify('preGenerate'))
+    it('in preProcessMdl', async () => verify('preProcessMdl'))
+    it('in postProcessMdl', async () => verify('postProcessMdl'))
+    it('in preGenerateC', async () => verify('preGenerateC'))
+    it('in postGenerateC', async () => verify('postGenerateC'))
+    it('in postGenerate', async () => verify('postGenerate'))
+    it('in postBuild', async () => verify('postBuild'))
+  })
+
+  // TODO: Not sure how to cause the preprocessor to fail, so this test is
+  // skipped for now
+  it.skip('should fail if preprocess step throws an error', async () => {
+    const modelSpec: ModelSpec = {
+      inputs: [{ varName: 'Y', defaultValue: 0, minValue: -10, maxValue: 10 }],
+      outputs: [{ varName: 'Z' }],
+      datFiles: []
+    }
+
+    const mdlDir = resolvePath(__dirname, '..', '_shared')
+    const userConfig: UserConfig = {
+      rootDir: resolvePath(__dirname, '..'),
+      prepDir: resolvePath(__dirname, 'sde-prep'),
+      modelFiles: [resolvePath(mdlDir, 'sample.mdl')],
+      modelSpec: async () => {
+        return modelSpec
+      }
+    }
+
+    const result = await build('production', buildOptions(userConfig))
+    if (result.isOk()) {
+      throw new Error('Expected err result but got: ' + result.value)
+    }
+
+    // TODO: This error message isn't helpful, but it's due to the fact that
+    // the `preprocess` function spawns an `sde` process rather than calling
+    // into the compiler directly.  Once we improve it to call into the
+    // compiler, we can improve the error message.
+    expect(result.error.message).toBe(`Failed to flatten mdl files: 'sde flatten' command failed (code=1)`)
+  })
+
+  it('should fail if flatten step throws an error', async () => {
+    const modelSpec: ModelSpec = {
+      inputs: [{ varName: 'Y', defaultValue: 0, minValue: -10, maxValue: 10 }],
+      outputs: [{ varName: 'Z' }],
+      datFiles: []
+    }
+
+    const mdlDir = resolvePath(__dirname, '..', '_shared')
+    const userConfig: UserConfig = {
+      rootDir: resolvePath(__dirname, '..'),
+      prepDir: resolvePath(__dirname, 'sde-prep'),
+      modelFiles: [resolvePath(mdlDir, 'submodel1.mdl'), resolvePath(mdlDir, 'submodel2.mdl')],
+      modelSpec: async () => {
+        return modelSpec
+      }
+    }
+
+    const result = await build('production', buildOptions(userConfig))
+    if (result.isOk()) {
+      throw new Error('Expected err result but got: ' + result.value)
+    }
+
+    // TODO: This error message isn't helpful, but it's due to the fact that
+    // the `flatten` function spawns an `sde` process rather than calling
+    // into the compiler directly.  Once we improve it to call into the
+    // compiler, we can improve the error message.
+    expect(result.error.message).toBe(`Failed to flatten mdl files: 'sde flatten' command failed (code=1)`)
+  })
+
+  it('should fail if generate step throws an error when dat file cannot be read', async () => {
+    const modelSpec: ModelSpec = {
+      inputs: [{ varName: 'Y', defaultValue: 0, minValue: -10, maxValue: 10 }],
+      outputs: [{ varName: 'Z' }],
+      datFiles: ['unknown.dat']
+    }
+
+    const userConfig: UserConfig = {
+      rootDir: resolvePath(__dirname, '..'),
+      prepDir: resolvePath(__dirname, 'sde-prep'),
+      modelFiles: [resolvePath(__dirname, '..', '_shared', 'sample.mdl')],
+      modelSpec: async () => {
+        return modelSpec
+      }
+    }
+
+    const result = await build('production', buildOptions(userConfig))
+    if (result.isOk()) {
+      throw new Error('Expected err result but got: ' + result.value)
+    }
+
+    // TODO: This error message isn't helpful, but it's due to the fact that
+    // the `generateC` function spawns an `sde` process rather than calling
+    // into the compiler directly.  Once we improve it to call into the
+    // compiler, the error message here should be the one from `readDat`.
+    expect(result.error.message).toBe(`Failed to generate C code: 'sde generate' command failed (code=1)`)
+  })
 })

packages/cli/src/sde-bundle.js

@@ -42,7 +42,8 @@ export let bundle = async (configPath, verbose) => {
     process.exit(result.exitCode)
   } else {
     // Exit with a non-zero code if any step failed
-    console.error(`ERROR: ${result.error.message}\n`)
+    console.error(result.error)
+    console.error()
     process.exit(1)
   }
 }

packages/cli/src/sde-dev.js

@@ -40,7 +40,8 @@ export let dev = async (configPath, verbose) => {
   // Exit with a non-zero code if any step failed, otherwise keep the
   // builder process alive
   if (!result.isOk()) {
-    console.error(`ERROR: ${result.error.message}\n`)
+    console.error(result.error)
+    console.error()
     process.exit(1)
   }
 }

packages/cli/src/sde-generate.js

@@ -43,8 +43,14 @@ export let builder = {
     alias: 'r'
   }
 }
-export let handler = argv => {
-  generate(argv.model, argv)
+export let handler = async argv => {
+  try {
+    await generate(argv.model, argv)
+  } catch (e) {
+    console.error(e)
+    console.error()
+    process.exit(1)
+  }
 }
 
 export let generate = async (model, opts) => {

packages/compile/src/_shared/helpers.js

@@ -375,7 +375,3 @@ export let vlog = (title, value, depth = 1) => {
     console.trace()
   }
 }
-export let abend = error => {
-  console.error(error)
-  process.exit(1)
-}