Southclaws
commented
1 year ago Funnily enough one of the main motivations for building Fault was sql.ErrNoRows breaking some rules around error handling.
short history note...
Basically, by using that type in error checks, we're leaking the underlying database details to the service layer with "SQL" and tying the concept of "not found" to only SQL related queries. If something was "not found" when querying MongoDB, Elasticsearch or S3 that means (by following this pattern) we need 4 custom error types and 4 checks in every handler to translate these errors into HTTP status codes. And thus, Fault's ftag (originally called errtag) was born.
end of history note
With regards to withSql I like the idea and I think it's very useful. This use-case is exactly why Fault is designed with the simple func(error) error pattern. And I'm happy to see the idea providing value!
However, I'm hesitant to add this to the library as it's quite use-case specific (raw sql usage without a library, some teams use ORMs, or Ent, or a different relational/non-relational database system.) And I'd imagine many teams wouldn't want all parameters of queries, which may contain sensitive information, be stored and potentially accessed for logging etc.
However, I think it would be good to add a section to the readme that links libraries that work well with Fault's wrapper interface.
I think there is an advantage to have common error types directly in the fault library so that other libraries can throw a common fault that can be extracted from the error chain consistently. I created such a wrapper that captures the SQL statement and parameters.
https://github.com/spearson78/fsql/blob/main/fsql.go
I propose that that withSql struct and the corresponding wrap and with methods be added to fault
This way other code that execute SQL can capture them and they can be logged consistently.