feat: carry structured location and exception cause in api errors

src/include/mx/api/Result.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <cassert>
+#include <exception>
 #include <optional>
 #include <string>
 #include <utility>
@@ -31,18 +32,56 @@ enum class ResultCode
     tooManyElements,
     invalidDocument,
     unsupportedVersion, // mirrored from the core parse boundary
-    // TODO: this badly swallows exception information. we need more variants
-    // or something like a site and message.
-    internalError, // caught exception; nothing escapes    (api-level)
+    outOfMemory,        // the machine ran out of memory        (api-level)
+    internalError,      // any other caught exception            (api-level)
+};
+
+// Where an error happened, as far as mx knows. An error may know the place
+// in the XML (xmlPath, byteOffset), the place in the music (part, measure,
+// staff, voice, and time), or neither. Unknown values are -1 for the
+// numbers and empty for the path.
+struct Location
+{
+    // The path through the XML elements to where the error is, e.g.
+    // /score-partwise/part[1]/measure[3]/note[2]
+    std::string xmlPath;
+
+    // Zero-based positions in the score. -1 means mx does not know.
+    int partIndex = -1;
+    int measureIndex = -1;
+    int staffIndex = -1;
+    int voiceIndex = -1;
+
+    // When in the music the error is, counted in ticks the way
+    // ScoreData::ticksPerQuarter describes. -1 means mx does not know.
+    int tickTimePosition = -1;
+
+    // How many bytes into the raw XML the error is. This is set when the XML
+    // itself could not be parsed. -1 means mx does not know.
+    long long byteOffset = -1;
 };
 
 struct ApiError
 {
     ResultCode code = ResultCode::internalError;
-    std::string path; // e.g. /score-partwise/part[1]/measure[3]/note[2]
+
+    // Where the error happened, where mx knows it.
+    Location location;
+
+    // A human-readable description of what went wrong.
     std::string message;
+
+    // When mx catches an exception, the exception itself is kept here so a
+    // caller can look at it or throw it again. Most errors are not
+    // exceptions, and this is null for those.
+    std::exception_ptr cause = nullptr;
 };
 
+// Writes an error on one line, the same way every time, so that logs and
+// test output are consistent and easy to read.
+// e.g. "mx: tooManyElements at part=0 measure=0: at most 8 beam occurrences"
+std::string formatError(const ApiError &error);
+
 // A small expected-like result, the same shape as mx::core's Result.
 template <typename T> class Result
 {

src/private/mx/api/MusicXml.cpp

@@ -70,12 +70,18 @@ ResultCode mirrorToApiResultCode(core::ErrorCode code)
 
 ApiError mirrorToApiError(const core::Error &error)
 {
-    return ApiError{mirrorToApiResultCode(error.code), error.path, error.message};
+    Location location;
+    location.xmlPath = error.path;
+    return ApiError{mirrorToApiResultCode(error.code), location, error.message};
 }
 
-ApiError musicXmlInternalError(const char *function, const std::string &message)
+// Builds the error for a caught exception. Call it inside a catch block only:
+// current_exception() keeps the exception alive in `cause`, so a caller can
+// look at it or throw it again. Most entry points repeat the same three
+// catches (out of memory, known exception, anything else).
+ApiError caughtError(const char *function, ResultCode code, const std::string &message)
 {
-    return ApiError{ResultCode::internalError, "", std::string{function} + ": " + message};
+    return ApiError{code, Location{}, std::string{function} + ": " + message, std::current_exception()};
 }
 
 std::string musicXmlFileExtension(const std::string &filePath)
@@ -150,16 +156,19 @@ Result<MusicXml> MusicXml::fromFile(const std::string &filePath)
             if (loaded.status == pugi::status_file_not_found || loaded.status == pugi::status_io_error ||
                 loaded.status == pugi::status_out_of_memory)
             {
-                return ApiError{ResultCode::ioError, filePath, loaded.description()};
+                return ApiError{ResultCode::ioError, Location{},
+                                "could not read '" + filePath + "' (" + loaded.description() + ")"};
             }
+            Location location;
+            location.byteOffset = loaded.offset;
             if (musicXmlFileExtension(filePath) == "mxl")
             {
                 std::stringstream ss;
                 ss << "it looks like you are trying to parse a compressed musicxml file, which is currently "
                    << "unsupported. https://github.com/webern/mx/issues/66 (" << loaded.description() << ")";
-                return ApiError{ResultCode::xmlSyntaxError, filePath, ss.str()};
+                return ApiError{ResultCode::xmlSyntaxError, location, ss.str()};
             }
-            return ApiError{ResultCode::xmlSyntaxError, filePath, loaded.description()};
+            return ApiError{ResultCode::xmlSyntaxError, location, loaded.description()};
         }
 
         auto parsed = core::parse(xdoc);
@@ -170,13 +179,17 @@ Result<MusicXml> MusicXml::fromFile(const std::string &filePath)
 
         return MusicXml{core::Document{std::move(parsed).value()}, true};
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("MusicXml::fromFile", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("MusicXml::fromFile", e.what());
+        return caughtError("MusicXml::fromFile", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("MusicXml::fromFile", "unknown exception");
+        return caughtError("MusicXml::fromFile", ResultCode::internalError, "unknown exception");
     }
 }
 
@@ -188,7 +201,9 @@ Result<MusicXml> MusicXml::fromStream(std::istream &stream)
         const pugi::xml_parse_result loaded = xdoc.load(stream, pugi::parse_default | pugi::parse_doctype);
         if (!loaded)
         {
-            return ApiError{ResultCode::xmlSyntaxError, "", loaded.description()};
+            Location location;
+            location.byteOffset = loaded.offset;
+            return ApiError{ResultCode::xmlSyntaxError, location, loaded.description()};
         }
 
         auto parsed = core::parse(xdoc);
@@ -199,13 +214,17 @@ Result<MusicXml> MusicXml::fromStream(std::istream &stream)
 
         return MusicXml{core::Document{std::move(parsed).value()}, true};
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("MusicXml::fromStream", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("MusicXml::fromStream", e.what());
+        return caughtError("MusicXml::fromStream", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("MusicXml::fromStream", "unknown exception");
+        return caughtError("MusicXml::fromStream", ResultCode::internalError, "unknown exception");
     }
 }
 
@@ -225,17 +244,21 @@ Result<void> MusicXml::writeToFile(const std::string &filePath) const
         }
         if (!xdoc.save_file(filePath.c_str(), "  "))
         {
-            return ApiError{ResultCode::ioError, filePath, "writeToFile: could not write the file"};
+            return ApiError{ResultCode::ioError, Location{}, "writeToFile: could not write '" + filePath + "'"};
         }
         return Result<void>{};
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("MusicXml::writeToFile", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("MusicXml::writeToFile", e.what());
+        return caughtError("MusicXml::writeToFile", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("MusicXml::writeToFile", "unknown exception");
+        return caughtError("MusicXml::writeToFile", ResultCode::internalError, "unknown exception");
     }
 }
 
@@ -256,13 +279,17 @@ Result<void> MusicXml::writeToStream(std::ostream &stream) const
         xdoc.save(stream, "  ");
         return Result<void>{};
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("MusicXml::writeToStream", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("MusicXml::writeToStream", e.what());
+        return caughtError("MusicXml::writeToStream", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("MusicXml::writeToStream", "unknown exception");
+        return caughtError("MusicXml::writeToStream", ResultCode::internalError, "unknown exception");
     }
 }
 
@@ -304,13 +331,17 @@ Result<MusicXml> fromScore(const ScoreData &score)
         // model will not represent.
         return refusal.error();
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("fromScore", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("fromScore", e.what());
+        return caughtError("fromScore", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("fromScore", "unknown exception");
+        return caughtError("fromScore", ResultCode::internalError, "unknown exception");
     }
 }
 
@@ -334,13 +365,17 @@ Result<ScoreData> getScore(const MusicXml &document)
         impl::ScoreReader reader{coreDocument.asScorePartwise()};
         return reader.getScoreData();
     }
+    catch (const std::bad_alloc &)
+    {
+        return caughtError("getScore", ResultCode::outOfMemory, "out of memory");
+    }
     catch (const std::exception &e)
     {
-        return musicXmlInternalError("getScore", e.what());
+        return caughtError("getScore", ResultCode::internalError, e.what());
     }
     catch (...)
     {
-        return musicXmlInternalError("getScore", "unknown exception");
+        return caughtError("getScore", ResultCode::internalError, "unknown exception");
     }
 }
 

src/private/mx/api/Result.cpp

@@ -0,0 +1,96 @@
+// MusicXML Class Library
+// Copyright (c) by Matthew James Briggs
+// Distributed under the MIT License
+
+#include "mx/api/Result.h"
+
+namespace mx
+{
+namespace api
+{
+std::string formatError(const ApiError &error)
+{
+    const char *codeName = "internalError";
+    switch (error.code)
+    {
+    case ResultCode::ioError:
+        codeName = "ioError";
+        break;
+    case ResultCode::xmlSyntaxError:
+        codeName = "xmlSyntaxError";
+        break;
+    case ResultCode::unknownElement:
+        codeName = "unknownElement";
+        break;
+    case ResultCode::unknownAttribute:
+        codeName = "unknownAttribute";
+        break;
+    case ResultCode::missingRequiredElement:
+        codeName = "missingRequiredElement";
+        break;
+    case ResultCode::missingRequiredAttribute:
+        codeName = "missingRequiredAttribute";
+        break;
+    case ResultCode::wrongElementOrder:
+        codeName = "wrongElementOrder";
+        break;
+    case ResultCode::tooManyElements:
+        codeName = "tooManyElements";
+        break;
+    case ResultCode::invalidDocument:
+        codeName = "invalidDocument";
+        break;
+    case ResultCode::unsupportedVersion:
+        codeName = "unsupportedVersion";
+        break;
+    case ResultCode::outOfMemory:
+        codeName = "outOfMemory";
+        break;
+    case ResultCode::internalError:
+        codeName = "internalError";
+        break;
+    }
+
+    std::string text{"mx: "};
+    text += codeName;
+
+    // the place in the document or the score, if known
+    std::string where;
+    const auto appendWhere = [&where](const char *name, long long value) {
+        if (value < 0)
+        {
+            return;
+        }
+        if (!where.empty())
+        {
+            where += ' ';
+        }
+        where += name;
+        where += '=';
+        where += std::to_string(value);
+    };
+
+    where += error.location.xmlPath;
+    appendWhere("part", error.location.partIndex);
+    appendWhere("measure", error.location.measureIndex);
+    appendWhere("staff", error.location.staffIndex);
+    appendWhere("voice", error.location.voiceIndex);
+    appendWhere("tick", error.location.tickTimePosition);
+    appendWhere("offset", error.location.byteOffset);
+
+    if (!where.empty())
+    {
+        text += " at ";
+        text += where;
+    }
+
+    if (!error.message.empty())
+    {
+        text += ": ";
+        text += error.message;
+    }
+
+    return text;
+}
+} // namespace api
+} // namespace mx

src/private/mx/impl/NoteWriter.cpp

@@ -65,6 +65,22 @@ core::Syllabic convertLyricSyllabicForNoteWriter(api::LyricSyllabic value)
     return core::Syllabic::single();
 }
 
+// The refusal sites below know where they are: the core error says why the
+// note was refused, and the cursor says where in the score the note sits.
+// Together they make an error a caller can act on -- the core error alone
+// does not say which note was the problem.
+WriteRefusal writeRefusalAt(const MeasureCursor &cursor, const core::Error &error)
+{
+    api::Location location;
+    location.xmlPath = error.path;
+    location.partIndex = cursor.partIndex;
+    location.measureIndex = cursor.measureIndex;
+    location.staffIndex = cursor.staffIndex;
+    location.voiceIndex = cursor.voiceIndex;
+    location.tickTimePosition = cursor.tickTimePosition;
+    return WriteRefusal{api::ApiError{api::ResultCode::tooManyElements, location, "NoteWriter: " + error.message}};
+}
+
 NoteWriter::NoteWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter,
                        bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes,
                        int inNumVoices, const std::string &inVoiceLabel)
@@ -160,8 +176,7 @@ core::Note NoteWriter::getNote(bool isStartOfChord) const
         {
             // Refuse, don't drop: the core caps
             // beams at 8; silently discarding the ninth would lose data.
-            throw WriteRefusal{api::ApiError{api::ResultCode::tooManyElements, added.error().path,
-                                             "NoteWriter: " + added.error().message}};
+            throw writeRefusalAt(myCursor, added.error());
         }
         ++beamIndex;
     }
@@ -294,8 +309,7 @@ void NoteWriter::assembleNoteChoice() const
                 const auto added = inner.addTie(tie);
                 if (!added)
                 {
-                    throw WriteRefusal{api::ApiError{api::ResultCode::tooManyElements, added.error().path,
-                                                     "NoteWriter: " + added.error().message}};
+                    throw writeRefusalAt(myCursor, added.error());
                 }
             }
             choiceObj.setGraceNoteChoice(core::GraceNoteChoice::graceNormalNoteGroup(std::move(inner)));
@@ -319,8 +333,7 @@ void NoteWriter::assembleNoteChoice() const
             const auto added = choiceObj.addTie(tie);
             if (!added)
             {
-                throw WriteRefusal{api::ApiError{api::ResultCode::tooManyElements, added.error().path,
-                                                 "NoteWriter: " + added.error().message}};
+                throw writeRefusalAt(myCursor, added.error());
             }
         }
         myOutNote.setChoice(core::NoteChoice::normalNoteGroup(std::move(choiceObj)));