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.

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

Panic while formatting an error when Pg connection is reset #1255

Closed
jethrogb opened this Issue Oct 11, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@jethrogb
Contributor

jethrogb commented Oct 11, 2017

Problem Description

When a Postgres database connection is closed unexpectedly, trying to debug-format the error returned by Diesel panics. Here's a simple test program (using a table named config):

fn main() {
    let connection = establish_connection();
    loop {
        println!("{:?}", config::table.count().first::<i64>(&connection));
        std::thread::sleep(std::time::Duration::from_secs(1));
    }
}

Have the DB server running before starting the program. After you see an Ok(...) line, forcefully terminate the DB server, for example using kill -9. I'm getting the following panic:

thread 'main' panicked at 'internal error: entered unreachable code: Per PGs documentation, all errors should have a message', .cargo/registry/src/github.com-1ecc6299db9ec823/diesel-0.16.0/src/pg/connection/result.rs:97:20
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: <diesel::pg::connection::result::PgErrorInformation as diesel::result::DatabaseErrorInformation>::message
             at .cargo/registry/src/github.com-1ecc6299db9ec823/diesel-0.16.0/src/pg/connection/result.rs:97
   8: <diesel::result::DatabaseErrorInformation + core::marker::Sync + core::marker::Send + 'static as core::fmt::Debug>::fmt
             at .cargo/registry/src/github.com-1ecc6299db9ec823/diesel-0.16.0/src/result.rs:52
   9: <alloc::boxed::Box<T> as core::fmt::Debug>::fmt
             at /checkout/src/liballoc/boxed.rs:534
  10: <&'a T as core::fmt::Debug>::fmt
             at /checkout/src/libcore/fmt/mod.rs:1482
  11: core::fmt::write
             at /checkout/src/libcore/fmt/mod.rs:951
  12: core::fmt::builders::DebugTuple::field
             at /checkout/src/libcore/fmt/builders.rs:0
  13: <diesel::result::Error as core::fmt::Debug>::fmt
             at .cargo/registry/src/github.com-1ecc6299db9ec823/diesel-0.16.0/src/result.rs:6
  14: <&'a T as core::fmt::Debug>::fmt
             at /checkout/src/libcore/fmt/mod.rs:1482
  15: core::fmt::write
             at /checkout/src/libcore/fmt/mod.rs:951
  16: core::fmt::builders::DebugTuple::field
             at /checkout/src/libcore/fmt/builders.rs:0
  17: <core::result::Result<T, E> as core::fmt::Debug>::fmt
             at /checkout/src/libcore/result.rs:250
  18: core::fmt::write
             at /checkout/src/libcore/fmt/mod.rs:951
  19: <std::io::stdio::Stdout as std::io::Write>::write_fmt
             at /checkout/src/libstd/io/mod.rs:1117
             at /checkout/src/libstd/io/stdio.rs:461
  20: std::io::stdio::_print
             at /checkout/src/libstd/io/stdio.rs:679
             at /checkout/src/libstd/io/stdio.rs:701
  21: dtest::main
             at src/main.rs:27
  22: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  23: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:458
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:59
  24: main
  25: __libc_start_main
  26: _start

Setup

Versions

  • Rust: 1.20
  • Diesel: 0.16.0
  • Database: CockroachDB 1.0
  • Operating System Ubuntu 16.04
  • libpq: 9.5.8-0ubuntu0.16.04.1

Feature Flags

  • diesel: default
  • diesel_codegen: default

@jethrogb jethrogb changed the title from Panic formatting error when Postgres DB connection is closed unexpectedly to Panic while formatting an error when Pg connection is reset Oct 11, 2017

@sgrif sgrif added this to the 1.0 milestone Oct 11, 2017

@sgrif

This comment has been minimized.

Member

sgrif commented Oct 11, 2017

This is definitely a blocker for 1.0

@sgrif

This comment has been minimized.

Member

sgrif commented Oct 11, 2017

Hm... I get this when testing locally: Err(DatabaseError(__Unknown, "terminating connection due to unexpected postmaster exit"))

@jethrogb

This comment has been minimized.

Contributor

jethrogb commented Oct 11, 2017

That error is coming from the postgres backend (backend/libpq/be-secure.c), something must be intercepting your kill signal. You might also try using tcpkill instead of killing the server process.

sgrif added a commit that referenced this issue Oct 16, 2017

Ensure PG results always have an error message
In certain cases (for example, if the database connection unexpectedly
terminated), we will have a `PGResult` pointer which is not null, but
does not have an error response from the server (and therefore no result
error fields). In this case, PG will copy the connection's error message
onto the result. However, this needs to be retrieved with a separate
function, as it is not part of an error response from the server.

I'd like to explore if there are any other cases that can result in
this, and potentially look at providing a specific `DatabaseErrorKind`
variant for this case.

Fixes #1255.

sgrif added a commit that referenced this issue Oct 16, 2017

Ensure PG results always have an error message
In certain cases (for example, if the database connection unexpectedly
terminated), we will have a `PGResult` pointer which is not null, but
does not have an error response from the server (and therefore no result
error fields). In this case, PG will copy the connection's error message
onto the result. However, this needs to be retrieved with a separate
function, as it is not part of an error response from the server.

I'd like to explore if there are any other cases that can result in
this, and potentially look at providing a specific `DatabaseErrorKind`
variant for this case.

Fixes #1255.

@sgrif sgrif closed this in #1260 Oct 17, 2017

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