Suggestion: Instead of throwing exception on errors returned from Typst library, return an error object

[jshergal]

I know that discriminated unions still don't exist in .NET, but since we are already returning a CompileOutcome object, it would be easy to extend that object to include the error information and then consumers of the library don't have to add additional exception handling to deal with control flow issues. Plus, with the update to return the error information (including line number and column info) from the Typst library, we could now create a more meaningful error list (we could even parse it to provide a TypstError object that has line and column info.

[jshergal]

@evolvedlight - I wanted to further this proposal along and try to flesh out the idea a little to see what you think.

Add TypstCompileError record:

sealed record TypstCompileError(string Message, int Line, int Column, string TextHint)
{
    public static TypstCompileError(string typstErrorMessage)
    {
        // parse typst error string via regex, extracting line, column and hint if available
    }
}

Update the CompileOutcome record to include the error collection:

public sealed record CompileOutcome(IReadOnlyList<byte[]> Buffers, IReadOnlyList<string> Warnings, IReadOnlyList<TypstCompileError> Errors);

Then in the code where we currently throw an exception, we can split the error message by lines and build the collection of errors. From the consumer side, if the Error list is non-empty, then there were errors and now they have clean access to those errors, including line and column information, without having to parse the error string themselves. If we wanted to include the raw error string in the error record, we could, but I don't know if that would be valuable or not.

Another option, in keeping with the current behavior of throwing an exception, would be to create a custom TypstCompileErrorException and then the exception itself could contain the list of TypstCompileError. Personally I would lean away from that, just to avoid the exceptions as control flow problem, but it does keep the code flow basically the same as it is now, with the only change being the type of exception thrown. While this would still be a breaking change, it wouldn't be as large, and it would then at least provide the ability for the consumer to handle the compilation error exceptions separately.

[CodesInChaos]

At least for my use-cases an exception is more convenient. A compiler error indicates a bug (typst templates effectively are part of our codebase), and an producing an exception for those automatically ends up the appropriate error handler.

But of course that's different for applications where the typst code is user controlled, and compiler errors are expected in normal operation. For those an exception wouldn't be appropriate.

Perhaps the exception could be moved to accessing result.Buffers. That way a caller can check the errors in the compiler output, but a simple caller who just consumes the output will get a meaningful exception. Alternatively a different compile method (e.g. TryCompile) could make sense.

[jshergal]

@CodesInChaos - in keeping with the exception route, how would you feel about a more specific type of exception? Something like TypstCompileErrorException that would have a member property Errors that would be a ReadOnlyList<TypstCompileError>, which would then give you the line, column, etc. and would allow for at least more granular exception handling coming from the compiler.

I do like the idea of a maybe adding a TryCompile method that would keep the current behavior for the compile methods, but for someone who doesn't want to use the exceptions for control flow, they could just run TryCompile and then pull the error information from the CompileOutcome.

[CodesInChaos]

The exception case should definitely use specific exceptions. I'd probably use a TypstException as base (e.g. for the unknown error case), and a TypstCompilationException derived from it containing a list of errors.