Currently the logging isn't that great, it uses fmt.Printf("xxx\n") where a error is being logged with no prefix or any idea which function called it.
This PR adds a logging module that is able to log to multiple sources, will display the date + time + filename + caller/linenumber of the function that called the log.XXX(), this should make the log file on the server-side more fun to read and easier to understand why something has errored.
I did touch some other lines not related to logging to make it more idiomatic.
- Currently the logging isn't that great, it uses fmt.Printf("xxx\n") where a error is being logged with no prefix or any idea which function called it.
- This PR adds a logging module that is able to log to multiple sources, will display the date + time + filename + caller/linenumber of the function that called the log.XXX(), this should make the log file on the server-side more fun to read and easier to understand why something has errored.
- I did touch some other lines not related to logging to make it more idiomatic.
- Currently the logging isn't that great, it uses fmt.Printf("xxx\n")
where a error is being logged with no prefix or any idea which function
called it.
- This PR adds a logging module that is able to log to multiple sources,
will display the date + time + filename + caller/linenumber of the
function that called the log.XXX(), this should make the log file on the
server-side more fun to read and easier to understand why something has
errored.
- I did touch some other lines not related to logging to make it more idiomatic.
Thank you. Your PR doesn't include the more recent commit 04c5f3c40e that already heavily touched error handling. Please use this as a base, because it shifts error handling + logging to one place.
If there are left over print statements, they should be updated to use this approach if possible.
Thank you. Your PR doesn't include the more recent commit https://codeberg.org/Codeberg/moderation/commit/04c5f3c40edb3fe0961986a29ddd20ee5a162e2b that already heavily touched error handling. Please use this as a base, because it shifts error handling + logging to one place.
If there are left over print statements, they should be updated to use this approach if possible.
Thank you. Your PR doesn't include the more recent commit 04c5f3c40e that already heavily touched error handling. Please use this as a base, because it shifts error handling + logging to one place.
If there are left over print statements, they should be updated to use this approach if possible.
Yeah I just noticed... I'm not sure if the creative approach is idiomatic or good. But I will try to use it as base for now.
> Thank you. Your PR doesn't include the more recent commit https://codeberg.org/Codeberg/moderation/commit/04c5f3c40edb3fe0961986a29ddd20ee5a162e2b that already heavily touched error handling. Please use this as a base, because it shifts error handling + logging to one place.
>
> If there are left over print statements, they should be updated to use this approach if possible.
Yeah I just noticed... I'm not sure if the creative approach is idiomatic or good. But I will try to use it as base for now.
I've tried to be in-line of that commit. I had to adjust it a bit in order to have have the call hierachy correct when log.XXX will be called so the correct function + line/filename can be shown in the log.
The commit also a includes for the shuffling of the slice(doing a random int of the slice has a good chance that the random int will land onto something that already has been "searched" for).
I've tried to be in-line of that commit. I had to adjust it a bit in order to have have the call hierachy correct when log.XXX will be called so the correct function + line/filename can be shown in the log.
The commit also a includes for the shuffling of the slice(doing a random int of the slice has a good chance that the random int will land onto something that already has been "searched" for).
https://codeberg.org/Codeberg/moderation/src/branch/main/backend/main.go#L117 I think this is unnecessary as of this line. Sorry, I missed this when I added it myself.
by the way, you proposed to me to rename this just to blah.Config(). I somehow thought I already changed it, but looks like I didn't yet. Feel free to update this if you'd like.
by the way, you proposed to me to rename this just to blah.Config(). I somehow thought I already changed it, but looks like I didn't yet. Feel free to update this if you'd like.
No. But I personally don't see the benefit of the status code when you can instead print the error. Because go-sdk should convert the status code(best-effort) to a more generalized error.
No. But I _personally_ don't see the benefit of the status code when you can instead print the error. Because go-sdk should convert the status code(best-effort) to a more generalized error.
I had an issue recently that I run code on already transferred repos, and the status code was 405 method not allowed. It didn't really help me with debugging, but the SDK message wasn't more helpful I think. I just hope that the improved error handling would have avoided this issue in the 1st place, so I don't object to leaving this as-is.
I had an issue recently that I run code on already transferred repos, and the status code was 405 method not allowed. It didn't really help me with debugging, but the SDK message wasn't more helpful I think. I just hope that the improved error handling would have avoided this issue in the 1st place, so I don't object to leaving this as-is.
Thank you. Your PR doesn't include the more recent commit
04c5f3c40e
that already heavily touched error handling. Please use this as a base, because it shifts error handling + logging to one place.If there are left over print statements, they should be updated to use this approach if possible.
Yeah I just noticed... I'm not sure if the creative approach is idiomatic or good. But I will try to use it as base for now.
I've tried to be in-line of that commit. I had to adjust it a bit in order to have have the call hierachy correct when log.XXX will be called so the correct function + line/filename can be shown in the log.
The commit also a includes for the shuffling of the slice(doing a random int of the slice has a good chance that the random int will land onto something that already has been "searched" for).
}
func ErrorMiddleware(fn func(w http.ResponseWriter, r *http.Request) error) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/json")
https://codeberg.org/Codeberg/moderation/src/branch/main/backend/main.go#L117 I think this is unnecessary as of this line. Sorry, I missed this when I added it myself.
if err != nil {
fmt.Printf("Failed to read file: %v\n", err)
os.Exit(1)
log.Fatal("ini.Load[file=%q]: %v", "moderation_backend.ini", err)
what's filled into the %q? Where does it take the value from?
moderation_backend.ini
is the value and%q
means print as quoted with"
. But given this is hard-coded it should be removed.ah oh, sorry, I overlooked the second parameter. I thought it was one long string and err and wondered where the second value should be coming from :)
mailer.MailConfig(cfg)
userConfig(cfg)
repoConfig(cfg)
log.LogConfig(cfg)
by the way, you proposed to me to rename this just to blah.Config(). I somehow thought I already changed it, but looks like I didn't yet. Feel free to update this if you'd like.
}
_, res, err := giteaClient.TransferRepo(currentOwner, currentName, gitea.TransferRepoOption{
_, _, err := giteaClient.TransferRepo(currentOwner, currentName, gitea.TransferRepoOption{
is the result status code included in the error?
No. But I personally don't see the benefit of the status code when you can instead print the error. Because go-sdk should convert the status code(best-effort) to a more generalized error.
Ref:
a56a62a4df/gitea/client.go (L303)
I had an issue recently that I run code on already transferred repos, and the status code was 405 method not allowed. It didn't really help me with debugging, but the SDK message wasn't more helpful I think. I just hope that the improved error handling would have avoided this issue in the 1st place, so I don't object to leaving this as-is.
2b47c34a12
into main 3 months ago2b47c34a12
.