Throw ObjectDisposedException on assignment to Disposed NpgsqlCommand - #6048
#6048Throw ObjectDisposedException on assignment to Disposed NpgsqlCommand#6048
Conversation
|
But why? Almost always you're not supposed to interact with a disposed object, especially changing a field there. |
|
I agree it doesn't really make much sense to modify after disposed. The primary reason I'm proposing the change is so interaction with a disposed connection don't throw Would you prefer it differentiate between Disposed and InProgress and throw ObjectDisposedException or InvalidOperationException accordingly? |
|
I would rather we just throw an ObjectDisposedException here. I'd be happy to merge that change. |
|
@kevbot18 friendly ping. Could you adapt your changes to make the code throw an ObjectDisposedException? |
c1813eb to
001aa5f
Compare
001aa5f to
c842a87
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling when properties are assigned to a disposed NpgsqlCommand by throwing an ObjectDisposedException instead of the previous "An open data reader exists" error message.
Key Changes:
- Added explicit disposal state checks in
CommandTextandConnectionproperty setters - Throws
ObjectDisposedExceptionwhen attempting to assign to these properties on a disposed command
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Npgsql/NpgsqlCommand.cs | Updated CommandText setter to use switch statement and Connection setter to use switch expression, both checking for CommandState.Disposed |
| test/Npgsql.Tests/CommandTests.cs | Added test to verify ObjectDisposedException is thrown when assigning to disposed command properties |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Checks for Disposed state when assigning Connection or CommandText on NpgsqlCommand and throws an ObjectDisposedException if disposed, instead of raising an open data reader exception.