Skip to content
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

improve message for authentication errors #105

Merged
merged 3 commits into from
Feb 25, 2021
Merged

Conversation

anuragsoni
Copy link
Contributor

@anuragsoni anuragsoni commented Feb 24, 2021

During the startup message flow postgres will shutdown the socket immediately after sending an error frame. Trying to read further from the socket will always result in EOF. If we instead exit early by raising the exception with Error_response, the error message seen by pgx users will be a little better. For a login failure, before this PR we would see something like:

(monitor.ml.Error (Pgx_async.Pgx_eof)
 ("Raised at Pgx_async.Thread.input_char.(fun) in file \"pgx_async/src/pgx_async.ml\", line 50, characters 14-27"
  "Called from Async_kernel__Deferred1.M.map.(fun) in file \"src/deferred1.ml\", line 17, characters 40-45"
  "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 167, characters 6-47"))

With the diff in this PR the error will show up as:

(monitor.ml.Error
 (pgx/src/types.ml.PostgreSQL_Error
  "Failed to authenticate with postgres server"
  ((code 28000) (severity FATAL)
   (message "Peer authentication failed for user \"faa\"")
   (custom ((R auth_failed) (L 330) (F auth.c) (V FATAL)))))
 ("Raised at Pgx.Make.connect.(fun).loop.(fun) in file \"pgx/src/pgx.ml\", line 645, characters 8-85"
  "Called from Async_kernel__Deferred0.bind.(fun) in file \"src/deferred0.ml\", line 54, characters 64-69"
  "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 167, characters 6-47"))

@coveralls
Copy link

Coverage Status

Coverage remained the same at 66.848% when pulling 2917b46 on improve-error-reporting into e408363 on master.

@anuragsoni anuragsoni merged commit 8d5ca02 into master Feb 25, 2021
@anuragsoni anuragsoni deleted the improve-error-reporting branch February 25, 2021 15:40
brendanlong added a commit to brendanlong/opam-repository that referenced this pull request May 12, 2021
…x_lwt_unix, pgx_lwt and pgx (2.0)

CHANGES:

### Breaking changes

* The Pgx module is now wrapped, which means `Pgx_aux`, `Types`, `Access`, etc. aren't added to the global scope.
  The main result of this is that `Pgx_value` now needs to be accessed as `Pgx.Value`.
  (arenadotio/pgx#103)
* `Pgx_async.connect` and `with_conn` now have an additional optional `?ssl` argument (see below).

### Added

* Pgx_async now supports TLS connections using Conduit_async. This is enabled by default and can be controlled with the
  new `?ssl` argument to `connect` and `with_conn`.
  (arenadotio/pgx#108)

### Fixed

* Improved message for authentication errors. Previously these raised `Pgx_eof`, and now they raise
  `PostgreSQL_Error("Failed to authenticate with postgres server", additional details ...)`.
  (arenadotio/pgx#105)

### Changed

* Support new Mirage-conduit timeout argument (arenadotio/pgx#95).
yomimono pushed a commit to yomimono/pgx that referenced this pull request Mar 8, 2022
* improve message for authentication errors

* use newer opam images for build

* add upper bound on conduit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants