InvalidOperationException instead of NpgsqlException when client disconnects · Issue #5605 · npgsql/npgsql · GitHub
Skip to content

InvalidOperationException instead of NpgsqlException when client disconnects #5605

Description

@pggh

Steps to reproduce

using System;
using System.Threading;
using Npgsql;

class Test2
{
    static int Main(string[] args)
    {
        var connectionString = new NpgsqlConnectionStringBuilder {
            Host = "test-server",
            Database = "postgres",
            Username = "postgres",
            Password = "123456",
            Pooling = false,
            CommandTimeout = 0,
            KeepAlive = 30,
            TcpKeepAlive = true,
            TcpKeepAliveTime = 30
        };
        var connection = new NpgsqlConnection();
        connection.ConnectionString = connectionString.ConnectionString;
        connection.Open();

        try {
            using (var c = new NpgsqlCommand("SELECT 'dummy'", connection)) {
                c.ExecuteNonQuery();
            }
            Console.Write("disconnect the this client from the network and press enter when done so...");
            // note: you can also work on localhost and stop the server instead of disconnecting the network
            Console.ReadLine();
            Console.WriteLine("waiting 1 minute.");
            Thread.Sleep(60_000);
            Console.WriteLine("BeginTransaction");
            connection.BeginTransaction();
        }
        catch (NpgsqlException ex) {
            Console.WriteLine("Expected Exception: " + Environment.NewLine + ex);
        }
        catch (Exception unexpected) {
            Console.WriteLine("Unexpected Exception:" + Environment.NewLine + unexpected);
        }

        return 0;
    }
}

The issue

According to the doc, exceptions related to connection errors should be wrapped in a NpgsqlException exception.
This makes perfectly sense as this will allow catching NpgsqlException at a relatively high level in the code,
dispose all DB related stuff and trying to create a new connection.

However in the example above, an InvalidOperationException will be thrown; this is definitely not an exception that should be handled and ignored at a higher level.
I think it is incorrect to throw a InvalidOperationException and it should be NpgsqlException with possibly containing the IOException. (If the connection would be used again after handling the NpgsqlException, InvalidOperationException would be correct.)

System.InvalidOperationException: Connection is not open
   at Npgsql.ThrowHelper.ThrowInvalidOperationException(String message)
   at Npgsql.NpgsqlConnection.CheckReady()
   at Npgsql.NpgsqlConnection.BeginTransaction(Boolean async, IsolationLevel level, CancellationToken cancellationToken)
   at Npgsql.NpgsqlConnection.BeginTransaction(IsolationLevel level)
   at Npgsql.NpgsqlConnection.BeginTransaction()
   at Test2.Main(String[] args) in Test2.cs:line 35

Further technical details

Npgsql version: 8.0.2
PostgreSQL version: 16
Operating system: Windows, .NET 6

Activity

  1. NinoFloris commented on Mar 12, 2024

    @NinoFloris
    Member

    This sounds reasonable, I've marked it as an enhancement.

    We won't have access to the reason the connection was broken (in this case due to keepalive) but it'll be good to throw the right type. The message should contain some pointers to check the logs etc.

  2. iliyas86 commented on Apr 18, 2025

    @iliyas86

    Is there any plans to fix this issue?

  3. added this to the 10.0.0 milestone on Apr 18, 2025
  4. added theissue type on Jun 14, 2025
  5. modified the milestones: 10.0.0, Backlog on Nov 2, 2025
  6. irrationalmentality commented on Jun 1, 2026

    @irrationalmentality

    Hi! I'd like to take this one.

    Following up on @NinoFloris's note above: my plan is to stop treating a Broken
    connection the same as a Closed one in the connection state checks
    (CheckReady / CheckOpen), and throw an NpgsqlException instead of
    InvalidOperationException when the connection is in the Broken state.

    Two things I'd like to confirm before opening a PR:

    1. When the original exception that broke the connector is still available
      (e.g. the IOException from a keepalive failure), should it be attached as
      the InnerException?
    2. Is there anything beyond CheckReady / CheckOpen you'd want covered here?

    I'll follow up with a draft PR including a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions