浏览代码

fix(error handling): Fix error loading empty score, improve error handling (#367)

* improve error handling for measure reading, voices, undefined values.
enable rendering empty score

fix #358

* small error message improvements

* logging: info instead of warn on measure without voices

* OSMD: standard score name "Unknown score" instead of "Unknown path"

MusicSheetReader: handle error reading title or composer when no page height given

demo: one common error function for all loading/rendering errors

* refactor(VexFlowStaffEntry): replace try catch with formatted check for calculateXPosition

* fix(MusicSheetReader): separate try block into two again so second method won't be skipped

if readTitleAndComposerFromCredits fails

* demo: always StackTrace, not checking for environment variable anymore
Simon 6 年之前
父节点
当前提交
aa65792eb1

+ 13 - 5
demo/index.js

@@ -146,7 +146,7 @@ import { OpenSheetMusicDisplay } from '../src/OpenSheetMusicDisplay/OpenSheetMus
                 try {
                     openSheetMusicDisplay.render();
                 } catch (e) {
-                    console.warn(e.stack);
+                    errorLoadingOrRenderingSheet(e, "rendering");
                 }
                 enable();
             }
@@ -233,20 +233,28 @@ import { OpenSheetMusicDisplay } from '../src/OpenSheetMusicDisplay/OpenSheetMus
                 return openSheetMusicDisplay.render();
             },
             function(e) {
-                console.warn(e.stack);
-                error("Error reading sheet: " + e);
+                errorLoadingOrRenderingSheet(e, "rendering");
             }
         ).then(
             function() {
                 return onLoadingEnd(isCustom);
             }, function(e) {
-                console.warn(e.stack);
-                error("Error rendering sheet: " + process.env.DEBUG ? e.stack : e);
+                errorLoadingOrRenderingSheet(e, "loading");
                 onLoadingEnd(isCustom);
             }
         );
     }
 
+    function errorLoadingOrRenderingSheet(e, loadingOrRenderingString) {
+        var errorString = "Error " + loadingOrRenderingString + " sheet: " + e;
+        // if (process.env.DEBUG) { // people may not set a debug environment variable for the demo.
+        // Always giving a StackTrace might give us more and better error reports.
+        // TODO for a release, StackTrace control could be reenabled
+        errorString += "\n" + "StackTrace: \n" + e.stack;
+        // }
+        console.warn(errorString);
+    }
+
     function onLoadingEnd(isCustom) {
         sampleLoaded = true;
         // Remove option from select

+ 1 - 0
external/vexflow/vexflow.d.ts

@@ -96,6 +96,7 @@ declare namespace Vex {
             public x_shift: number;
             public getAbsoluteX(): number;
             public addModifier(index: number, modifier: Modifier): StemmableNote;
+            public preFormatted: boolean;
         }
 
         export class GhostNote extends StemmableNote {

+ 3 - 0
src/MusicalScore/Graphical/VexFlow/VexFlowMeasure.ts

@@ -644,6 +644,9 @@ export class VexFlowMeasure extends GraphicalMeasure {
         const voices: Voice[] = this.getVoicesWithinMeasure();
 
         for (const voice of voices) {
+            if (voice === undefined) {
+                continue;
+            }
             const isMainVoice: boolean = !(voice instanceof LinkedVoice);
 
             // add a vexFlow voice for this voice:

+ 1 - 1
src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -118,7 +118,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
             }
         }
         if (voices.length === 0) {
-            log.warn("Found a measure with no voices... Continuing anyway.", mvoices);
+            log.info("Found a measure with no voices. Continuing anyway.", mvoices);
             continue;
         }
         // all voices that belong to one stave are collectively added to create a common context in VexFlow.

+ 3 - 0
src/MusicalScore/Graphical/VexFlow/VexFlowStaffEntry.ts

@@ -29,6 +29,9 @@ export class VexFlowStaffEntry extends GraphicalStaffEntry {
         for (const gve of this.graphicalVoiceEntries as VexFlowVoiceEntry[]) {
             if (gve.vfStaveNote) {
                 gve.vfStaveNote.setStave(stave);
+                if (!gve.vfStaveNote.preFormatted) {
+                    continue;
+                }
                 gve.applyBordersFromVexflow();
                 this.PositionAndShape.RelativePosition.x = gve.vfStaveNote.getBoundingBox().x / unitInPixels;
                 const sourceNote: Note = gve.notes[0].sourceNote;

+ 18 - 9
src/MusicalScore/ScoreIO/MusicSheetReader.ts

@@ -64,6 +64,7 @@ export class MusicSheetReader /*implements IMusicSheetReader*/ {
             return this._createMusicSheet(root, path);
         } catch (e) {
             log.info("MusicSheetReader.CreateMusicSheet", e);
+            return undefined;
         }
     }
 
@@ -486,21 +487,24 @@ export class MusicSheetReader /*implements IMusicSheetReader*/ {
     private pushSheetLabels(root: IXmlElement, filePath: string): void {
         this.readComposer(root);
         this.readTitle(root);
-        if (this.musicSheet.Title === undefined || this.musicSheet.Composer === undefined) {
-            this.readTitleAndComposerFromCredits(root);
+        try {
+            if (this.musicSheet.Title === undefined || this.musicSheet.Composer === undefined) {
+                this.readTitleAndComposerFromCredits(root); // this can also throw an error
+            }
+        } catch (ex) {
+            log.info("MusicSheetReader.pushSheetLabels", "readTitleAndComposerFromCredits", ex);
         }
-        if (this.musicSheet.Title === undefined) {
-            try {
+        try {
+            if (this.musicSheet.Title === undefined) {
                 const barI: number = Math.max(
                     0, filePath.lastIndexOf("/"), filePath.lastIndexOf("\\")
                 );
                 const filename: string = filePath.substr(barI);
                 const filenameSplits: string[] = filename.split(".", 1);
                 this.musicSheet.Title = new Label(filenameSplits[0]);
-            } catch (ex) {
-                log.info("MusicSheetReader.pushSheetLabels: ", ex);
             }
-
+        } catch (ex) {
+            log.info("MusicSheetReader.pushSheetLabels", "read title from file name", ex);
         }
     }
 
@@ -611,8 +615,13 @@ export class MusicSheetReader /*implements IMusicSheetReader*/ {
         }
         let paperHeight: number = 0;
         let topSystemDistance: number = 0;
-        const defi: string = root.element("defaults").element("page-layout").element("page-height").value;
-        paperHeight = parseFloat(defi);
+        try {
+            const defi: string = root.element("defaults").element("page-layout").element("page-height").value;
+            paperHeight = parseFloat(defi);
+        } catch (e) {
+            log.info("MusicSheetReader.computeSystemYCoordinates(): couldn't find page height, not reading title/composer.");
+            return 0;
+        }
         let found: boolean = false;
         const parts: IXmlElement[] = root.elements("part");
         for (let idx: number = 0, len: number = parts.length; idx < len; ++idx) {

+ 5 - 1
src/OpenSheetMusicDisplay/OpenSheetMusicDisplay.ts

@@ -137,7 +137,11 @@ export class OpenSheetMusicDisplay {
         const score: IXmlElement = new IXmlElement(elem);
         const calc: MusicSheetCalculator = new VexFlowMusicSheetCalculator();
         const reader: MusicSheetReader = new MusicSheetReader();
-        this.sheet = reader.createMusicSheet(score, "Unknown path");
+        this.sheet = reader.createMusicSheet(score, "Untitled Score");
+        if (this.sheet === undefined) {
+            // error loading sheet, probably already logged, do nothing
+            return Promise.reject(new Error("given music sheet was incomplete or could not be loaded."));
+        }
         this.graphic = new GraphicalMusicSheet(this.sheet, calc);
         if (this.drawingParameters.drawCursors) {
             this.cursor.init(this.sheet.MusicPartManager, this.graphic);