Always provide an meaningful exception message when CSV parsing fails #269
Conversation
When parsing failed the exception message was previous the generic "Exception of type 'Sylvan.Data.Csv.CsvFormatException' was thrown", which gave no clue as to what was wrong. Provide a good exception message in each case where this exception is thrown, including the position where parsing failed, a snippet of the input (which can be searched for to locate the problematic input easily) and what was wrong with it. Update unit tests to check for the new exception messages.
| @@ -7,8 +7,8 @@ namespace Sylvan.Data.Csv; | |||
| /// </summary> | |||
| public class CsvFormatException : FormatException | |||
| { | |||
| internal CsvFormatException(int row, int ordinal, string? msg = null, Exception? inner = null) | |||
| : base(msg, inner) | |||
| internal CsvFormatException(int row, int ordinal, string message, Exception? inner = null) | |||
There was a problem hiding this comment.
Made message mandatory (non-nullable), so that any future uses of this exception would require a message to be passed in as well.
I would say there may be a want to omit snippets via configuration, for sensitivity in log compliance. Should make that configurable. |
That's a good point, though it does complicate things a little. If the maintainer wants this then I think I'd rather change this PR to not include a snippet of the input at all, then try to add that in a later PR. |
When parsing failed the exception message was previous the generic "Exception of type 'Sylvan.Data.Csv.CsvFormatException' was thrown", which gave no clue as to what was wrong. Provide a good exception message in each case where this exception is thrown, including the position where parsing failed, a snippet of the input (which can be searched for to locate the problematic input easily) and what was wrong with it. Update unit tests to check for the new exception messages.