Merge pull request #47 from lstreckeisen/CMI-95-Improve-Language-Server-Command-error-handling

make generator throw error when something goes wrong

Author: lstreckeisen

src/language/commands/GeneratorCommandExecutor.ts

@@ -14,40 +14,34 @@ export class GeneratorCommandExecutor {
     this.serviceRegistry = serviceRegistry
   }
 
-  async execute (generator: ContextMapperGenerator, args: unknown[], cancelToken: CancellationToken): Promise<string[] | undefined> {
+  async execute (generator: ContextMapperGenerator, args: unknown[], cancelToken: CancellationToken): Promise<string[]> {
     const filePath = args[0] as string
 
     const model = await this.extractModel(filePath)
     if (cancelToken.isCancellationRequested) {
-      return undefined
+      return []
     }
 
-    if (model == null) {
-      return undefined
-    }
-    args.shift()
+    args.shift() // remove source file from args array
     return await generator.generate(model, filePath, args, cancelToken)
   }
 
-  private async extractModel (filePath: string): Promise<ContextMappingModel | undefined> {
+  private async extractModel (filePath: string): Promise<ContextMappingModel> {
     const extensions = ContextMapperDslLanguageMetaData.fileExtensions as readonly string[]
     if (!extensions.includes(path.extname(filePath))) {
-      console.error('Unsupported file extension on file', filePath)
-      return undefined
+      throw new Error(`Unsupported file extension on file ${filePath}`)
     }
 
     if (!fs.existsSync(filePath)) {
-      console.error(`File ${filePath} does not exist.`)
-      return undefined
+      throw new Error(`File ${filePath} does not exist.`)
     }
 
     const document = await this.getServices().shared.workspace.LangiumDocuments.getOrCreateDocument(URI.file(path.resolve(filePath)))
     await this.getServices().shared.workspace.DocumentBuilder.build([document], { validation: true })
 
     const validationErrors = (document.diagnostics ?? []).filter(e => e.severity === 1)
     if (validationErrors.length > 0) {
-      console.error(`File ${filePath} is invalid`)
-      return undefined
+      throw new Error(`File ${filePath} is invalid`)
     }
 
     return document.parseResult.value as ContextMappingModel

src/language/commands/generators/ContextMapperGenerator.ts

@@ -2,5 +2,5 @@ import { ContextMappingModel } from '../../generated/ast.js'
 import { CancellationToken } from 'vscode-languageserver'
 
 export interface ContextMapperGenerator {
-  generate(model: ContextMappingModel, filePath: string, args: unknown[], cancelToken: CancellationToken): Promise<string[] | undefined>
+  generate(model: ContextMappingModel, filePath: string, args: unknown[], cancelToken: CancellationToken): Promise<string[]>
 }

src/language/commands/generators/PlantUMLGenerator.ts

@@ -6,16 +6,16 @@ import { ComponentDiagramGenerator } from './plantuml/ComponentDiagramGenerator.
 import { CancellationToken } from 'vscode-languageserver'
 
 export class PlantUMLGenerator implements ContextMapperGenerator {
-  async generate (model: ContextMappingModel, filePath: string, args: unknown[], cancelToken: CancellationToken): Promise<string[] | undefined> {
+  async generate (model: ContextMappingModel, filePath: string, args: unknown[], cancelToken: CancellationToken): Promise<string[]> {
     // there must not be any extra spaces especially at the start, since the path will be treated as relative otherwise
     const destination = (args[0] as string)?.trim()
     if (destination == null || destination === '') {
       console.log('Destination must be specified')
-      return undefined
+      throw Error('Destination must be specified')
     }
 
     if (cancelToken.isCancellationRequested) {
-      return undefined
+      return []
     }
     const fileName = filePath.split('/').pop()!.split('.')[0]
 
@@ -30,7 +30,6 @@ export class PlantUMLGenerator implements ContextMapperGenerator {
       diagrams.push(componentDiagram)
     }
 
-    console.log('Successfully generated PlantUML diagrams')
     return diagrams
   }
 

test/commands/CommandExecution.test.ts

@@ -5,6 +5,7 @@ import { afterEach, beforeAll, describe, expect, test } from 'vitest'
 import { NodeFileSystem } from 'langium/node'
 import fs from 'node:fs'
 import path from 'node:path'
+import { fail } from 'node:assert'
 
 const outDir = path.join(__dirname, 'out')
 
@@ -38,8 +39,12 @@ describe('Command execution tests', () => {
   test('test plantUML command with invalid file extension', async () => {
     const file = path.join(__dirname, '..', 'invalid-files', 'test.txt')
 
-    const result = await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
-    expect(result).toBeUndefined()
+    try {
+      await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
+      fail('Expected generator to fail')
+    } catch (e) {
+      expect(e).not.toBeUndefined()
+    }
 
     const outContentExists = fs.existsSync(outDir)
     expect(outContentExists).toEqual(false)
@@ -48,8 +53,12 @@ describe('Command execution tests', () => {
   test('test plantUML command with invalid file', async () => {
     const file = path.join(__dirname, '..', 'invalid-files', 'invalid.cml')
 
-    const result = await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
-    expect(result).toBeUndefined()
+    try {
+      await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
+      fail('Expected generator to fail')
+    } catch (e) {
+      expect(e).not.toBeUndefined()
+    }
 
     const outContentExists = fs.existsSync(outDir)
     expect(outContentExists).toEqual(false)
@@ -58,8 +67,12 @@ describe('Command execution tests', () => {
   test('test plantUML command with non-existing file', async () => {
     const file = path.join(__dirname, '..', 'invalid-files', 'non-existing.cml')
 
-    const result = await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
-    expect(result).toBeUndefined()
+    try {
+      await commandHandler.executeCommand('org.contextmapper.GeneratePlantUML', [file, outDir])
+      fail('Expected generator to fail')
+    } catch (e) {
+      expect(e).not.toBeUndefined()
+    }
 
     const outContentExists = fs.existsSync(outDir)
     expect(outContentExists).toEqual(false)