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

Add fault for sql queries and parameters #21

Open
spearson78 opened this issue Nov 24, 2022 · 1 comment
Open

Add fault for sql queries and parameters #21

spearson78 opened this issue Nov 24, 2022 · 1 comment
Labels
enhancement New feature or request question Further information is requested

Comments

@spearson78
Copy link

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.

@Southclaws
Copy link
Owner

Southclaws commented Nov 29, 2022

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.

@Southclaws Southclaws added enhancement New feature or request question Further information is requested labels Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants