feat: carry structured location and exception cause in api errors - #436
Open
webern wants to merge 1 commit into
Open
feat: carry structured location and exception cause in api errors#436webern wants to merge 1 commit into
webern wants to merge 1 commit into
Conversation
ApiError now says where and keeps what. Location holds the place in the XML (element path, byte offset) or in the score (part, measure, staff, voice, tick); the WriteRefusal sites stamp it from the cursor already in scope. std::bad_alloc reaching the boundary is reported as ResultCode::outOfMemory; every other caught exception is internalError with the exception itself kept in std::exception_ptr cause. formatError renders an error the same way every time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
TODO: human writes here
Summary
internalErrorwas a dead end: it swallowed the exception type, said nothing about where the problem was, and aTODOinResult.hsaid so. This givesApiErrortwo things it was missing: a place and a cause.Locationsays where, in whichever world the error happened:cause(std::exception_ptr) keeps the actual exception that was caught, so a caller can rethrow it, inspect its type, or read its message;std::bad_allocnow maps to its ownResultCode::outOfMemoryinstead of hiding ininternalError. The catch blocks inMusicXml.cppcapturestd::current_exception()intocause.Two sources of position are wired now, without threading any new context through the call stack:
WriteRefusalsites inNoteWriterwere already holding aMeasureCursor(part, measure, staff, voice, tick) when they refused a note; the error now carries it. A ninth beam comes back astooManyElements at part=0 measure=0 staff=0 voice=0 tick=0.formatError(const ApiError&)renders an error the same way every time (mx: tooManyElements at part=0 measure=0: ...), for logs, tests, and anyone (or anything) reading test output.The
pathfield is replaced bylocation; aggregate initialization ofApiErrorchanges shape, so this is breaking, but only for code that constructs errors directly.mx::coreis untouched: the core mirror mapsError::pathintolocation.xmlPathand stays lossless.This is groundwork for #432: a warnings mechanism wants somewhere to say where and something to attach; this interface now has both.
Testing
ResultTest(new):Locationdefaults,formatErroroutput pinned for bare codes, XML paths, score coordinates, and byte offsetstooManyElementsCarriesThePlaceInTheScore: a 9-beam note refuses with the full cursor position, rendered byformatErroroutOfMemoryIsReportedNotThrown: astd::bad_allocthrown mid-parse comes back asoutOfMemory, rethrowable fromcauseinternalErrorKeepsTheException: an unexpected exception comes back asinternalErrorwith the original type and message preserved incausemake api-roundtripregression: 414 passed, 0 failedmake fmt-checkpassesReferences