Skip to content

Use ThrowHelper to throw exceptions (part 1) #4872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vonzshik merged 5 commits into from
Jan 13, 2023
Merged

Use ThrowHelper to throw exceptions (part 1) #4872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
962eea3
Use ThrowHelper to throw exceptions (part 1)
vonzshik Jan 12, 2023
6ccdd5f
A few more ThrowHelper usages
vonzshik Jan 13, 2023
e510e04
Rewrite FullState
vonzshik Jan 13, 2023
4706764
Optimize ValidateParameterCount throw
vonzshik Jan 13, 2023
1e82a0d
Rewrite InTransaction
vonzshik Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/Npgsql/BackendMessages/RowDescriptionMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,11 @@ public FieldDescription this[int index]
/// Given a string name, returns the field's ordinal index in the row.
/// </summary>
internal int GetFieldIndex(string name)
=> TryGetFieldIndex(name, out var ret)
? ret
: throw new IndexOutOfRangeException("Field not found in row: " + name);
{
if (!TryGetFieldIndex(name, out var ret))
ThrowHelper.ThrowIndexOutOfRangeException($"Field not found in row: {name}");
return ret;
}

/// <summary>
/// Given a string name, returns the field's ordinal index in the row.
Expand Down Expand Up @@ -181,7 +183,14 @@ public Enumerator(RowDescriptionMessage rowDescription)
=> _rowDescription = rowDescription;

public FieldDescription Current
=> _pos >= 0 ? _rowDescription[_pos] : throw new InvalidOperationException();
{
get
{
if (_pos < 0)
ThrowHelper.ThrowInvalidOperationException();
Comment thread
vonzshik marked this conversation as resolved.
return _rowDescription[_pos];
}
}

object IEnumerator.Current => Current;

Expand Down
46 changes: 29 additions & 17 deletions src/Npgsql/Internal/NpgsqlConnector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ internal ValueTask<IBackendMessage> ReadMessage(bool async, DataRowLoadingMode d
var authType = (AuthenticationRequestType)buf.ReadInt32();
return authType switch
{
AuthenticationRequestType.AuthenticationOk => (AuthenticationRequestMessage)AuthenticationOkMessage.Instance,
AuthenticationRequestType.AuthenticationOk => AuthenticationOkMessage.Instance,
AuthenticationRequestType.AuthenticationCleartextPassword => AuthenticationCleartextPasswordMessage.Instance,
AuthenticationRequestType.AuthenticationMD5Password => AuthenticationMD5PasswordMessage.Load(buf),
AuthenticationRequestType.AuthenticationGSS => AuthenticationGSSMessage.Instance,
Expand Down Expand Up @@ -1540,14 +1540,23 @@ internal Task Rollback(bool async, CancellationToken cancellationToken = default
}

internal bool InTransaction
=> TransactionStatus switch
{
get
{
TransactionStatus.Idle => false,
TransactionStatus.Pending => true,
TransactionStatus.InTransactionBlock => true,
TransactionStatus.InFailedTransactionBlock => true,
_ => throw new InvalidOperationException($"Internal Npgsql bug: unexpected value {TransactionStatus} of enum {nameof(TransactionStatus)}. Please file a bug.")
};
switch (TransactionStatus)
{
case TransactionStatus.Idle:
return false;
case TransactionStatus.Pending:
case TransactionStatus.InTransactionBlock:
case TransactionStatus.InFailedTransactionBlock:
return true;
default:
ThrowHelper.ThrowInvalidOperationException($"Internal Npgsql bug: unexpected value {{0}} of enum {nameof(TransactionStatus)}. Please file a bug.", TransactionStatus);
return false;
}
}
}

/// <summary>
/// Handles a new transaction indicator received on a ReadyForQuery message
Expand All @@ -1562,7 +1571,7 @@ void ProcessNewTransactionStatus(TransactionStatus newStatus)
switch (newStatus)
{
case TransactionStatus.Idle:
break;
return;
case TransactionStatus.InTransactionBlock:
case TransactionStatus.InFailedTransactionBlock:
// In multiplexing mode, we can't support transaction in SQL: the connector must be removed from the
Expand All @@ -1571,14 +1580,15 @@ void ProcessNewTransactionStatus(TransactionStatus newStatus)
if (Connection is null)
{
Debug.Assert(Settings.Multiplexing);
throw new NotSupportedException("In multiplexing mode, transactions must be started with BeginTransaction");
ThrowHelper.ThrowNotSupportedException("In multiplexing mode, transactions must be started with BeginTransaction");
}
break;
return;
case TransactionStatus.Pending:
throw new Exception($"Internal Npgsql bug: invalid TransactionStatus {nameof(TransactionStatus.Pending)} received, should be frontend-only");
ThrowHelper.ThrowInvalidOperationException($"Internal Npgsql bug: invalid TransactionStatus {nameof(TransactionStatus.Pending)} received, should be frontend-only");
return;
default:
throw new InvalidOperationException(
$"Internal Npgsql bug: unexpected value {newStatus} of enum {nameof(TransactionStatus)}. Please file a bug.");
ThrowHelper.ThrowInvalidOperationException($"Internal Npgsql bug: unexpected value {{0}} of enum {nameof(TransactionStatus)}. Please file a bug.", newStatus);
return;
}
}

Expand Down Expand Up @@ -2172,7 +2182,8 @@ internal async Task Reset(bool async)
endBindingScope = true;
break;
default:
throw new InvalidOperationException($"Internal Npgsql bug: unexpected value {TransactionStatus} of enum {nameof(TransactionStatus)}. Please file a bug.");
ThrowHelper.ThrowInvalidOperationException($"Internal Npgsql bug: unexpected value {TransactionStatus} of enum {nameof(TransactionStatus)}. Please file a bug.");
return;
}

if (_sendResetOnClose)
Expand Down Expand Up @@ -2306,7 +2317,8 @@ UserAction DoStartUserAction(ConnectorState newState, NpgsqlCommand? command)
break;
case ConnectorState.Closed:
case ConnectorState.Broken:
throw new InvalidOperationException("Connection is not open");
ThrowHelper.ThrowInvalidOperationException("Connection is not open");
break;
case ConnectorState.Executing:
case ConnectorState.Fetching:
case ConnectorState.Waiting:
Expand All @@ -2318,7 +2330,7 @@ UserAction DoStartUserAction(ConnectorState newState, NpgsqlCommand? command)
? new NpgsqlOperationInProgressException(State)
: new NpgsqlOperationInProgressException(currentCommand);
default:
throw new ArgumentOutOfRangeException(nameof(State), State, "Invalid connector state: " + State);
throw new ArgumentOutOfRangeException(nameof(State), State, $"Invalid connector state: {State}");
}

Debug.Assert(IsReady);
Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Internal/NpgsqlReadBuffer.Stream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public override void Write(byte[] buffer, int offset, int count)
void CheckDisposed()
{
if (IsDisposed)
throw new ObjectDisposedException(null);
ThrowHelper.ThrowObjectDisposedException(nameof(ColumnStream));
}

protected override void Dispose(bool disposing)
Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Internal/NpgsqlReadBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ T Read<T>()
}

static void ThrowNotSpaceLeft()
=> throw new InvalidOperationException("There is not enough space left in the buffer.");
=> ThrowHelper.ThrowInvalidOperationException("There is not enough space left in the buffer.");

public string ReadString(int byteLen)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Internal/NpgsqlWriteBuffer.Stream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async Task WriteLong(byte[] buffer, int offset, int count, bool async, Cancellat
void CheckDisposed()
{
if (_disposed)
throw new ObjectDisposedException(null);
ThrowHelper.ThrowObjectDisposedException(nameof(ParameterStream));
}

protected override void Dispose(bool disposing)
Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/Internal/NpgsqlWriteBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ void Write<T>(T value)
}

static void ThrowNotSpaceLeft()
=> throw new InvalidOperationException("There is not enough space left in the buffer.");
=> ThrowHelper.ThrowInvalidOperationException("There is not enough space left in the buffer.");

public Task WriteString(string s, int byteLen, bool async, CancellationToken cancellationToken = default)
=> WriteString(s, s.Length, byteLen, async, cancellationToken);
Expand Down
8 changes: 4 additions & 4 deletions src/Npgsql/NpgsqlBinaryExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ async ValueTask<int> StartRow(bool async, CancellationToken cancellationToken =
if (_column == NumColumns)
_leftToReadInDataMsg = Expect<CopyDataMessage>(await _connector.ReadMessage(async), _connector).Length;
else if (_column != -1)
throw new InvalidOperationException("Already in the middle of a row");
ThrowHelper.ThrowInvalidOperationException("Already in the middle of a row");

await _buf.Ensure(2, async);
_leftToReadInDataMsg -= 2;
Expand Down Expand Up @@ -216,7 +216,7 @@ ValueTask<T> Read<T>(bool async, CancellationToken cancellationToken = default)
CheckDisposed();

if (_column == -1 || _column == NumColumns)
throw new InvalidOperationException("Not reading a row");
ThrowHelper.ThrowInvalidOperationException("Not reading a row");

var type = typeof(T);
var handler = _typeHandlerCache[_column];
Expand Down Expand Up @@ -267,7 +267,7 @@ ValueTask<T> Read<T>(NpgsqlDbType type, bool async, CancellationToken cancellati
{
CheckDisposed();
if (_column == -1 || _column == NumColumns)
throw new InvalidOperationException("Not reading a row");
ThrowHelper.ThrowInvalidOperationException("Not reading a row");

var handler = _typeHandlerCache[_column];
if (handler == null)
Expand Down Expand Up @@ -372,7 +372,7 @@ async Task ReadColumnLenIfNeeded(bool async)
void CheckDisposed()
{
if (_isDisposed)
throw new ObjectDisposedException(GetType().FullName, "The COPY operation has already ended.");
ThrowHelper.ThrowObjectDisposedException(nameof(NpgsqlBinaryExporter), "The COPY operation has already ended.");
}

#endregion
Expand Down
43 changes: 22 additions & 21 deletions src/Npgsql/NpgsqlCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,10 @@ public override string CommandText
{
Debug.Assert(!IsWrappedByBatch);

_commandText = State == CommandState.Idle
? value ?? string.Empty
: throw new InvalidOperationException("An open data reader exists for this command.");
if (State != CommandState.Idle)
ThrowHelper.ThrowInvalidOperationException("An open data reader exists for this command.");

_commandText = value ?? string.Empty;

ResetPreparation();
// TODO: Technically should do this also if the parameter list (or type) changes
Expand Down Expand Up @@ -827,7 +828,7 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
: (batchCommand.CommandText, batchCommand.CommandType, batchCommand.Parameters);

if (string.IsNullOrEmpty(commandText))
throw new InvalidOperationException("CommandText property has not been initialized");
ThrowHelper.ThrowInvalidOperationException("CommandText property has not been initialized");

switch (commandType)
{
Expand Down Expand Up @@ -862,7 +863,7 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr

case PlaceholderType.Named:
if (!EnableSqlRewriting)
throw new NotSupportedException($"Named parameters are not supported when Npgsql.{nameof(EnableSqlRewriting)} is disabled");
ThrowHelper.ThrowNotSupportedException($"Named parameters are not supported when Npgsql.{nameof(EnableSqlRewriting)} is disabled");

// The parser is cached on NpgsqlConnector - unless we're in multiplexing mode.
parser ??= new SqlQueryParser();
Expand All @@ -871,26 +872,27 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
{
parser.ParseRawQuery(this, standardConformingStrings);
if (InternalBatchCommands.Count > 1 && _parameters.HasOutputParameters)
throw new NotSupportedException("Commands with multiple queries cannot have out parameters");
ThrowHelper.ThrowNotSupportedException("Commands with multiple queries cannot have out parameters");
for (var i = 0; i < InternalBatchCommands.Count; i++)
ValidateParameterCount(InternalBatchCommands[i]);
}
else
{
parser.ParseRawQuery(batchCommand, standardConformingStrings);
if (batchCommand.Parameters.HasOutputParameters)
throw new NotSupportedException("Batches cannot cannot have out parameters");
ThrowHelper.ThrowNotSupportedException("Batches cannot cannot have out parameters");
ValidateParameterCount(batchCommand);
}

break;

case PlaceholderType.Mixed:
throw new NotSupportedException("Mixing named and positional parameters isn't supported");
ThrowHelper.ThrowNotSupportedException("Mixing named and positional parameters isn't supported");
break;

default:
throw new ArgumentOutOfRangeException(
nameof(PlaceholderType), $"Unknown {nameof(PlaceholderType)} value: {Parameters.PlaceholderType}");
ThrowHelper.ThrowArgumentOutOfRangeException(nameof(PlaceholderType), $"Unknown {nameof(PlaceholderType)} value: {Parameters.PlaceholderType}");
break;
}

break;
Expand Down Expand Up @@ -927,7 +929,7 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
if (parameter.IsPositional)
{
if (seenNamedParam)
throw new ArgumentException(NpgsqlStrings.PositionalParameterAfterNamed);
ThrowHelper.ThrowArgumentException(NpgsqlStrings.PositionalParameterAfterNamed);
}
else
{
Expand Down Expand Up @@ -958,14 +960,14 @@ internal void ProcessRawQuery(SqlQueryParser? parser, bool standardConformingStr
break;

default:
throw new InvalidOperationException($"Internal Npgsql bug: unexpected value {CommandType} of enum {nameof(CommandType)}. Please file a bug.");
ThrowHelper.ThrowInvalidOperationException($"Internal Npgsql bug: unexpected value {CommandType} of enum {nameof(CommandType)}. Please file a bug.");
break;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static void ValidateParameterCount(NpgsqlBatchCommand batchCommand)
{
if (batchCommand.PositionalParameters.Count > ushort.MaxValue)
throw new NpgsqlException($"A statement cannot have more than {ushort.MaxValue} parameters");
ThrowHelper.ThrowNpgsqlException("A statement cannot have more than 65535 parameters");
}
}

Expand Down Expand Up @@ -1315,7 +1317,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(CommandBehavior
{
Debug.Assert(conn is null);
if (behavior.HasFlag(CommandBehavior.CloseConnection))
throw new ArgumentException($"{nameof(CommandBehavior.CloseConnection)} is not supported with {nameof(NpgsqlConnector)}", nameof(behavior));
ThrowHelper.ThrowArgumentException($"{nameof(CommandBehavior.CloseConnection)} is not supported with {nameof(NpgsqlConnector)}", nameof(behavior));
connector = _connector;
}
else
Expand Down Expand Up @@ -1484,8 +1486,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(CommandBehavior
{
// The waiting on the ExecutionCompletion ManualResetValueTaskSource is necessarily
// asynchronous, so allowing sync would mean sync-over-async.
throw new NotSupportedException(
"Synchronous command execution is not supported when multiplexing is on");
ThrowHelper.ThrowNotSupportedException("Synchronous command execution is not supported when multiplexing is on");
}

if (IsWrappedByBatch)
Expand Down Expand Up @@ -1793,15 +1794,14 @@ public virtual NpgsqlCommand Clone()
return clone;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
NpgsqlConnection? CheckAndGetConnection()
{
if (State == CommandState.Disposed)
throw new ObjectDisposedException(GetType().FullName);
ThrowHelper.ThrowObjectDisposedException(GetType().FullName);
if (InternalConnection == null)
{
if (_connector is null)
throw new InvalidOperationException("Connection property has not been initialized.");
ThrowHelper.ThrowInvalidOperationException("Connection property has not been initialized.");
return null;
}
switch (InternalConnection.FullState)
Expand All @@ -1812,7 +1812,8 @@ public virtual NpgsqlCommand Clone()
case ConnectionState.Open | ConnectionState.Fetching:
return InternalConnection;
default:
throw new InvalidOperationException("Connection is not open");
ThrowHelper.ThrowInvalidOperationException("Connection is not open");
return null;
}
}

Expand Down
Loading