Skip to content

feat(codegen/golang): add an option to wrap query errors that includes query name #3876

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

Merged
merged 4 commits into from
Mar 30, 2025

Conversation

andrewmbenton
Copy link
Collaborator

This is an attempt to add wrapped errors to query functions in order to get the query name, as described in #1612.

If this approach looks reasonable I'll add more tests. But I'll also have to decide whether to wrap errors coming from the more esoteric function types like batch and copyfrom.

@kyleconroy
Copy link
Collaborator

I'd change the error message to be shorter. Here's the error message from os.ReadFile if a file doesn't exist

open hello: no such file or directory

so I'd change the error message to something like

query {{MethodName}}: %w

I also know that no matter what format we choose, people will want to change it.

@kyleconroy
Copy link
Collaborator

But I'll also have to decide whether to wrap errors coming from the more esoteric function types like batch and copyfrom.

Yeah, I'd be okay not wrapping errors from those functions simply because they have a larger surface area

@andrewmbenton
Copy link
Collaborator Author

Re error messages, agree people will want to change it and we should go as short as possible. I was mimicking what we do in dbCode e.g.

fmt.Errorf("error preparing query {{.MethodName}}: %w", err)

and

fmt.Errorf("error closing {{.FieldName}}: %w", cerr)

arguably those could be shorter as well though.

@andrewmbenton andrewmbenton marked this pull request as ready for review March 9, 2025 18:55
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. 🔧 golang labels Mar 9, 2025
@andrewmbenton
Copy link
Collaborator Author

Let me know if you think we need more tests, e.g. for configurations that branch in the templates in ways I might have missed.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 30, 2025
@kyleconroy kyleconroy merged commit 2f9e9d4 into main Mar 30, 2025
8 checks passed
@kyleconroy kyleconroy deleted the andrew/wrap-errors branch March 30, 2025 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files. 🔧 golang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants