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

Handle errors in a sane way #14

Open
davidwin93 opened this issue Jan 9, 2023 · 3 comments
Open

Handle errors in a sane way #14

davidwin93 opened this issue Jan 9, 2023 · 3 comments

Comments

@davidwin93
Copy link
Collaborator

davidwin93 commented Jan 9, 2023

Our current error handling should be improved to handle failures more gracefully. We currently retry failed attempts endlessly and we also endlessly retry Gitlab operations (update comment, etc...) . Adding some backoff with a max attempts would be a good start. We could also augment our Queue messages to maintain a count of attempts to prevent endless retries.

For NATS we could use some combo of these to manage message delivery:
https://pkg.go.dev/github.com/nats-io/nats.go#MaxDeliver
https://pkg.go.dev/github.com/nats-io/nats.go#Msg.Term
https://pkg.go.dev/github.com/nats-io/nats.go#Msg.Metadata

@sl1pm4t pointed to this https://choria.io/blog/post/2020/04/03/nats_patterns_9/ under Redelivery as a way of handling these types of messages.

@davidwin93
Copy link
Collaborator Author

davidwin93 commented Jan 9, 2023

This was fixed in release v0.1.4


Also my quick patch to fix the infinite loop introduced a bug

{"level":"debug","projectID":"--","mrIID":--,"time":"2023-01-09T20:28:40Z","message":"posting Gitlab comment"}
21
panic: runtime error: invalid memory address or nil pointer dereference
20
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xde53ed]
19
18
goroutine 62 [running]:
17
github.com/zapier/tfbuddy/pkg/gitlab_hooks.(*GitlabEventWorker).processNoteEvent(0xc000314200, {0x12be708, 0xc00028a000})
16
	/src/pkg/gitlab_hooks/comment_actions.go:85 +0x7ed
15
github.com/zapier/tfbuddy/pkg/gitlab_hooks.(*GitlabEventWorker).processNoteEventStreamMsg(0x1180?, 0xc00028a000?)
14
	/src/pkg/gitlab_hooks/hook_worker.go:41 +0x25
13
github.com/sl1pm4t/gongs.(*GenericStream[...]).QueueSubscribe.func1()
12
	/go/pkg/mod/github.com/sl1pm4t/[email protected]/generic_queue.go:70 +0x84
11
github.com/nats-io/nats%2ego.(*js).subscribe.func1(0x0?)
10
	/go/pkg/mod/github.com/nats-io/[email protected]/js.go:1639 +0x22
9
github.com/nats-io/nats%2ego.(*Conn).waitForMsgs(0xc0000ea000, 0xc000595040)
8
	/go/pkg/mod/github.com/nats-io/[email protected]/nats.go:2884 +0x442
7
created by github.com/nats-io/nats%2ego.(*Conn).subscribeLocked
6
	/go/pkg/mod/github.com/nats-io/[email protected]/nats.go:4114 +0x3f8

@davidwin93
Copy link
Collaborator Author

Given how critical it is to not fail on responding to hooks we should consider separating out hook ingestion to be standalone. This would guard against bugs/issues with the actual runner vs hook ingestion.

@sl1pm4t
Copy link
Contributor

sl1pm4t commented Jan 10, 2023

@davidwin93 I thought this too and started to split it out a few weeks ago, but shelved it while we worked on open sourcing. I can revisit this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants