Capturing SQL Exceptions #8

Open
docyes opened this Issue Jan 4, 2011 · 41 comments

Comments

Projects
None yet
6 participants

docyes commented Jan 4, 2011

Does a mechanism exist for capturing SQL exceptions and returning the error message in a http response?

Contributor

agentzh commented Jan 4, 2011

It's possible to use nginx's "error_page" to override the default 500/411 exceptions.

For more complicated cases, please check out the ngx_lua module: http://github.com/chaoslawful/lua-nginx-module

Basically you can issue a subrequest from within your Lua code to an internal nginx location configured by ngx_postgres, and then process the subrequest responses (like sql exceptions) the way you like in Lua ;)

-agentzh

Owner

PiotrSikora commented Jan 4, 2011

If you want to pass error message returned from PostgreSQL server, then no, it's not possible right now.

I'll add this to the TODO list (but there is no ETA).

docyes commented Sep 13, 2011

Just following up if you have any suggested patch paths I could take in the the source tree. My use case is failures for columns that are unique and detecting the error message on the consumer end (eg., username, email fields).

@docyes docyes closed this Sep 13, 2011

@docyes docyes reopened this Sep 13, 2011

Contributor

agentzh commented Sep 13, 2011

I think Piotr Sikora is working on $postgres_error_code and $postgres_error_message variables for this. Basically you can output these nginx variables via ngx_headers_more's more_set_headers, and capture these headers in Lua by inspecting the return values of ngx.location.capture or ngx.location.capture_multi in the context of ngx_lua :)

Owner

PiotrSikora commented Sep 13, 2011

What @agentzh meant is that I have this on my imminent (current or next week) TODO list :)

I want to add $postgres_error and $postgres_error_oneline (for pretty printing), so that you would be able to add those variables as headers using ngx_headers_more. Would that work for your use case or would you prefer to have errors printed in the response body?

docyes commented Sep 13, 2011

The $postgres_error and $postgres_error_oneline would definitely be useful and powerful. I do like your idea about having errors printed in the response body as I suspect a lot of users would love that feature including myself.

I've been using your module for www.geobacon.com and it's been rock solid.

Contributor

agentzh commented Sep 13, 2011

I wonder if it's feasible to encode such error code and error messages into the RDS output?

Owner

PiotrSikora commented Sep 13, 2011

@agentzh: error code & message are already present in the RDS header.

Contributor

agentzh commented Sep 13, 2011

@PiotrSikora Nice :)

Contributor

agentzh commented Sep 13, 2011

@docyes when PiotrSikora implemented the $postgres_error varialbe, you can append the error message to your main response body by using echo_after_body: http://wiki.nginx.org/HttpEchoModule#echo_after_body

Owner

PiotrSikora commented Sep 13, 2011

@agentzh: I thought about using ngx_echo for this, but how would echo_after_body differentiate error from non-error response?

Contributor

agentzh commented Sep 13, 2011

@docyes alternatively you can use ngx_lua + Lua + lua-rds-parser to generate a nice error page all by yourself now: see http://wiki.nginx.org/HttpLuaModule and https://github.com/agentzh/lua-rds-parser

Contributor

agentzh commented Sep 13, 2011

On Tue, Sep 13, 2011 at 5:28 PM, Piotr Sikora
reply@reply.github.com
wrote:

@agentzh: I thought about using ngx_echo for this, but how would echo_after_body differentiate error from non-error response?

That's a very good question :) We can do something like this as long
as the response status code can differentiate these two cases at
least:

location /foo {
    set $footer '';
    postgres_query ...;
    postgres_pass ...;
    header_filter_by_lua '
        if ngx.status ~= ngx.HTTP_OK then
            ngx.var.footer = "\\n\\n" .. ngx.var.postgres_error
        end
    ';
    echo_after_body $footer;
}

That is, appending empty string (in $footer) is a no-op for echo_after_body.

-agentzh

Owner

PiotrSikora commented Sep 13, 2011

Actually, it wasn't very good question :P

$postgres_error will be empty for non-error cases, so always appending it shouldn't be a problem.

docyes commented Sep 13, 2011

Thanks for looking into this you two, this is really valuable.

From my perspective I am really liking the idea from @agetzh
I wonder if it's feasible to encode such error code and error messages into the RDS output?

Having by default the appropriate http error status code along with RDS output of the error message would be sweet.

Owner

PiotrSikora commented Sep 13, 2011

@docyes: you must have missed part of the dialog. Error code & message are (and always been) present in the RDS output.

docyes commented Sep 14, 2011

Hey guys, can you confirm this is the recommended pattern.

Does the $postgres_error variable exist in the current release?

 server {
     listen       8000;
     server_name  localhost;
     location /query {
         echo_after_body  $postgres_error;
         postgres_pass  database;
         rds_json  on;
         postgres_query  "SELECT * FROM cats"
     }
  }
Contributor

agentzh commented Sep 14, 2011

I see the rds_json is turned on. It cannot display errstr message on your side directly? If not, maybe we should make it work rather than rely on the echo_after_body hack :)

docyes commented Sep 14, 2011

This is what i am seeing in my error.log file:
2011/09/14 00:09:59 [emerg] 8660#0: unknown "postgres_error" variable

docyes commented Sep 14, 2011

I'm not sure how difficult it would be to add the postgresql condition names? This would be a huge feature allowing developers to key of the condition value in their own internal lookups.

Here's a list of all the condition names:
http://www.postgresql.org/docs/9.0/static/errcodes-appendix.html

Owner

PiotrSikora commented Sep 14, 2011

@docyes:

Hey guys, can you confirm this is the recommended pattern.

It's not really "recommended", but yes, that represents the idea that we were talking about.

Does the $postgres_error variable exist in the current release?

No, it's not.

Also, it seems that you're confused by what RDS is... RDS (Resty-DBD-Stream) is common/semi-universal representation of relational responses (created by @agentzh and/or @chaoslawful), which we're using in both database modules (ngx_drizzle and ngx_postgres) to ease conversion process to more human-readable formats (JSON, CVS) and usage in different languages (C, Lua) regardless of the backend database.

The rds_json directive converts RDS to JSON, so you don't really "see" RDS.

I'm not sure how difficult it would be to add the postgresql condition names? This would be a huge feature allowing developers to key of the condition value in their own internal lookups.

Would it? Could you give me code snippet that would represent how use of this feature would look like?

@agentzh:

I see the rds_json is turned on. It cannot display errstr message on your side directly? If not, maybe we should make it work rather than rely on the echo_after_body hack :)

We're bailing-out on errors with return NGX_HTTP_INTERNAL_SERVER_ERROR (in both modules, AFAIR), so there is no RDS response in case of failure.

docyes commented Sep 14, 2011

@PiotrSikora
Sample http consumer using the postgresql condition name key:

import json
import simplerequest

response = simplrequest.post("/db/users", {'username': 'foobar'})
data = json.loads(response.text)
if data.get('error'):
    if data.get('code')=='unique_violation':
        return 'sorry that unsername already exists'
    else:
        return 'sorry we can't create users right now'
else:
     return 'your user account has been created'
Owner

PiotrSikora commented Sep 14, 2011

@docyes: Oh, I thought that you were talking about nginx.conf-side conditionals.

This is definitely doable and it makes a lot of sense (other than the fact that you really should use response.code >= 400 instead of data.get('error') as conditional), but it requires a bit of work and a bit of planning on how to nicely handle this in different formats and in ngx_drizzle without breaking anything.

Sadly, my "open-source time" is very limited lately, so I cannot give you any ETA on this... But don't expect it before November/December.

docyes commented Sep 14, 2011

@PiotrSikora
I think as a starting point, just getting the $postgres_error via the echo_after_body with the raw exception string is good enough if that's easier. I really appreciate your time and consideration on this.

Contributor

agentzh commented Sep 15, 2011

On Thu, Sep 15, 2011 at 2:15 AM, Piotr Sikora
reply@reply.github.com
wrote:

We're bailing-out on errors with return NGX_HTTP_INTERNAL_SERVER_ERROR (in both modules, AFAIR), so there is no RDS response in case of failure.

@PiotrSikora: Should we fix both ngx_postgres and ngx_drizzle such
that @docyes can process the RDS in such cases nicely? We could still
return 500 status code for backward-compatibility, but with a custom
error page body that is a valid RDS instance. And yeah, ngx_rds_json
and ngx_rds_csv needs to handle 500 too :)

Best,
-agentzh

Owner

PiotrSikora commented Sep 15, 2011

@agentzh: yes, we should... Eventually.

Just keep it mind that it shouldn't be enabled by default, at least not the verbose output, since it could leak potentially "secret" information.

docyes commented Sep 15, 2011

@PiotrSikora:
good point about the default mode of disabled in the event it's exposed outside of a locked down area.

leafo commented Nov 20, 2012

What's the status of this? I'm using ngx_postgres for a project and it would be nice to see errors in my application. I'm using Postgres 9.2. Whenever there is an error, my query location returns 500 error with no body.

Contributor

agentzh commented Nov 20, 2012

Hello!

On Tue, Nov 20, 2012 at 1:44 PM, leaf notifications@github.com wrote:

What's the status of this? I'm using ngx_postgres for a project and it would be nice to see errors in my application. I'm using Postgres 9.2. Whenever there is an error, my query location returns 500 error with no body.

I have a local patch for this and it mostly works. I'll try to polish
it up and commit it in the near future :)

Thanks for reminding me of this :)

Best regards,
-agentzh

@agentzh I was wondering the current behavior is. I have a use case where Postgres is correctly returning a duplicate key error, and ngx_postgres is returning a 500 back to the client. I'd like to be able to intercept this and return something in the range of 4xx to indicate to the client that the actual request data was wrong, as opposed to a server error. If the actual duplicate key message could get passed back to the client, that would be nice as well, as opposed to having it land in the openresty error log. Is there a way to achieve this?

leafo commented Nov 17, 2014

I've since written a pure Lua Postgres driver for OpenResty that you might find useful: https://github.com/leafo/pgmoon

@leafo Thanks for the heads up - do you have an example of where this is used in OpenResty to customize the error handling?

leafo commented Nov 17, 2014

If the query method fails then it returns nil and an error message. You're free to handle the error however you like: https://github.com/leafo/pgmoon#result-num_queries--postgresqueryquery_string

niquola commented Mar 12, 2015

@agentzh could you share your patch with $postgres_error var?

niquola commented Mar 12, 2015

Is possible to return error message as http header ?!

Contributor

agentzh commented Mar 12, 2015

@niquola AFAIK, the patch for ngx_postgres does not exist yet :)

niquola commented Mar 12, 2015

@agentzh could you give me a guidelines or similar examples - i'll try to do it by-myself

I'm mostly clojure, ruby guy (:
Нow to build ngx_postgres faster during development? - now i have to configure & make openresty build :(

Contributor

agentzh commented Mar 12, 2015

@niquola After the first build, you don't need to rerun ./configure for incremental changes in build/* (including ngx_postgres source there). Just run "make -j8 && sudo make install" and the build system will just pick up what has changed so it should be very fast. (If you have fewer available CPU logical cores than 8, try reducing that number in the -j option for "make").

Regarding exposing the Pg error message to an nginx variable, it should be an easy exercise for nginx C development. Just check out existing nginx C code for setting nginx variable values.

niquola commented Mar 12, 2015

Thx, I understand "this is 5 min task for you" - could you drop me a link
with example ;)
On 13 Mar 2015 02:19, "Yichun Zhang" notifications@github.com wrote:

@niquola https://github.com/niquola After the first build, you don't
need to rerun ./configure for incremental changes in build/* (including
ngx_postgres source there). Just run "make -j8 && sudo make install" and
the build system will just pick up what has changed so it should be very
fast. (If you have fewer available CPU logical cores than 8, try reducing
that number in the -j option for "make").

Regarding exposing the Pg error message to an nginx variable, it should be
an easy exercise for nginx C development. Just check out existing nginx C
code for setting nginx variable values.


Reply to this email directly or view it on GitHub
#8 (comment).

Contributor

agentzh commented Mar 13, 2015

@niquola You can check out how ngx_postgres implements the postgres_set directive itself.

niquola commented Mar 13, 2015

I've created pull-request - waiting for review.

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