allow composing Hooks

[ccmtaylor]

adds func Compose that allows composing multiple Hooks into one. It runs every callback on every hook in argument order, even if previous hooks return an error. If multiple hooks return errors, the error return value will be MultipleErrors, which allows for introspecting the errors if necessary.

I chose not to abort processing when any handler returns an error, because common usage would be to compose logging / tracing / metrics, and those kinds of handlers will want to run in every case to make sure that e.g. latencies are tracked correctly.

I added compatibility with Go 1.13's errors.Is/errors.As methods under a conditional compilation flag.

Fixes: #12

[qustavo]

@ccmtaylor do you know why CI is failing?

[jcchavezs]

I would say that logging or tracing or other signal collection should not interfere the output of the function as it should be as imperceivable as possible to the user.

ons. 20. nov. 2019, 14:50 skrev Keegan Carruthers-Smith < notifications@github.com>:

@keegancsmith commented on this pull request.

sorry for the slow turnaround, was on vacation the last two weeks :)

In compose.go https://github.com/gchaincl/sqlhooks/pull/25#discussion_r348462058:

• var errors []error
• for _, hook := range c {
• if onErrorer, ok := hook.(OnErrorer); ok {
• if err := onErrorer.OnError(ctx, cause, query, args...); err != nil && err != cause {
• errors = append(errors, err)
• }
• }
• }
• switch len(errors) {
• case 0:
• return cause
• case 1:
• return errors[0]
• default:
• return MultipleErrors(errors)
• }

Can you create a helper function which does this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/gchaincl/sqlhooks/pull/25?email_source=notifications&email_token=AAXOYAWBDPP4LKOEE5AJEJLQUUW77A5CNFSM4JHTZGIKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCMH7RVQ#pullrequestreview-319813846, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAWFRE43RQ3MOZU5QUTQUUW77ANCNFSM4JHTZGIA .

[keegancsmith]

Agreed now, the more I think about it the more I agree that the use case of composing hooks warrants the MultipleError behaviour. So LGTM.

[ccmtaylor]

@ccmtaylor do you know why CI is failing?

@gchaincl from the travis logs, it seems to be an issue with the postgres setup:

$ psql -c 'create database sqlhooks;' -U postgres
psql: could not connect to server: No such file or directory
    Is the server running locally and accepting
    connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?
The command "psql -c 'create database sqlhooks;' -U postgres" failed and exited with 2 during .

I'd be very surprised if it's related to these changes.

[qustavo]

Right, I think you should be using postgresql[1] in .travis.yml. I had the same problem and I changed it[2].

[1] https://github.com/gchaincl/sqlhooks/blob/master/.travis.yml#L4 [2] https://github.com/gchaincl/sqlhooks/commit/a494b89347123b3eecbd063569a25eb723595566

[coveralls]

Coverage increased (+3.9%) to 76.834% when pulling f55ee8fd87468b5458a13e527240270a22ef5794 on ccmtaylor:compose into a494b89347123b3eecbd063569a25eb723595566 on gchaincl:master.

[qustavo]

@ccmtaylor I saw that the CI is failing, might be related to https://github.com/mattn/go-sqlite3/issues/755 ?

[qustavo]

Hey @ccmtaylor I'm closing this one in favor of #28 which fixes the CI issue