Skip to content

Conversation

@MarkPflug
Copy link
Owner

@MarkPflug MarkPflug commented Jan 16, 2025

Reading after an exception is thrown will not advance the row number. Subsequent calls to Read will also throw, but a different exception type.

Potential fix for #262

… Subsequent calls to Read will also throw, but a different exception.
}

[Fact]
public void Exception()
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loop-evgeny Take a look at this test and let me know if you think it covers the issue you reported in #262

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it does - thanks!

else
{
// TODO:
throw new InvalidOperationException("The current row bla blah TODO");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not throw pendingException here instead, which contains more information?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pending exception was already thrown the previous time Read was called. I'm trying to indicate that it is no longer valid to call Read, the reader is in an error state and can no longer be used. I need to write an actual useful error message, hence the TODO.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. I think you're right about this one. I was thinking about the other InvalidOperationException that is thrown from subsequent Read calls.

@MarkPflug MarkPflug merged commit 23b60de into main Jan 19, 2025
1 check passed
@MarkPflug MarkPflug deleted the line_number branch January 19, 2025 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants