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

Handling PG exceptions #193

Closed
palkan opened this issue May 6, 2021 · 3 comments · Fixed by #194
Closed

Handling PG exceptions #193

palkan opened this issue May 6, 2021 · 3 comments · Fixed by #194

Comments

@palkan
Copy link
Owner

palkan commented May 6, 2021

From #69:

I'd also appreciate if there were an error_handling function which I could define to 'rescue' any errors inside logidze without failing the transaction. I do this for a number of my other function, e.g.

  EXCEPTION
    -- https://www.postgresql.org/docs/11/errcodes-appendix.html
    -- https://www.postgresql.org/docs/11/plpgsql-control-structures.html#PLPGSQL-ERROR-TRAPPING
    WHEN DEADLOCK_DETECTED OR QUERY_CANCELED OR LOCK_NOT_AVAILABLE THEN
      RAISE WARNING 'APP_NOTICE: DATABASE ERROR DETECTED: function % on invoice_ids %. running: %\n', fn_name, invoice_ids, current_query();
      GET STACKED DIAGNOSTICS
          _db_err_sql_state := RETURNED_SQLSTATE,
          _db_err_message := MESSAGE_TEXT,
          _db_err_detail := PG_EXCEPTION_DETAIL,
          _db_err_hint := PG_EXCEPTION_HINT,
          _db_err_context := PG_EXCEPTION_CONTEXT;

      IF COALESCE(current_setting('logidze.meta', true), '') <> '' THEN
        _db_err_responsible_id := current_setting('logidze.meta')::text;
      END IF;

      INSERT INTO database_errors (
        sql_state, message, detail, hint, context,
        fn_name, fn_args, current_query, responsible_id
      ) VALUES (
        _db_err_sql_state, _db_err_message, _db_err_detail, _db_err_hint, _db_err_context,
        fn_name, quote_literal(invoice_ids::text), current_query()::text, _db_err_responsible_id
      );
@palkan
Copy link
Owner Author

palkan commented May 6, 2021

@bf4 Could you, please, clarify the "without failing the transaction"? Do you want a trigger function to quietly fail (and log the error somehow) even if the log entry hasn't been created? How would this affect the consistency of data?

@bf4
Copy link
Contributor

bf4 commented May 6, 2021

re: "without failing the transaction"

It's my understanding that a function with raises an exception, whether it's a before or after trigger, causes the entire transaction to fail.

@palkan I have two ideas here

  1. have a no-op exception handler which allows user-defined exception handling*. ( perhaps allow it to be configured to fail with a warning instead of exception?) In my case, I'd log rescue it and log it in a database table, which will record the failed change, but allow me to review it at least. I'm not positive I could retry it in a useful way. (Some functions I do retry from that table). In general, losing log data is preferable to failing the action. I try never to raise from a side-effect.
  2. have a fallback 'serialization' strategy for known exception types which may be slower but are known or expected not to fail

either can be configured to take the name of a function and pass in args to it, or just be expected to be redefinable by the user

@palkan
Copy link
Owner Author

palkan commented May 7, 2021

or just be expected to be redefinable by the user

Yeah, we though about similar: defining a blank (or re-raising) logidze_exception_handler function and allow users to redefine it.

have a fallback 'serialization' strategy for known exception types which may be slower but are known or expected not to fail

I like it. We can extract diff-to-jsonb conversion into a separate function (oh no, one more function 🤯) and implement a fallback within it. So instead of

IF (coalesce(current_setting('logidze.full_snapshot', true), '') = 'on') THEN
changes = hstore_to_jsonb_loose(hstore(NEW.*));
ELSE
changes = hstore_to_jsonb_loose(
hstore(NEW.*) - hstore(OLD.*)
);
END IF;

We gonna use:

      IF (coalesce(current_setting('logidze.full_snapshot', true), '') = 'on') THEN
        changes = logidze_diff_to_jsonb(NEW.*);
      ELSE
        changes = logidze_diff_to_jsonb(NEW.*, OLD.*);
      END IF;

@skryukov WDYT?

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

Successfully merging a pull request may close this issue.

2 participants