Registry.Gather should turn panics into errors

[bboreham]

This code:

client_golang/prometheus/registry.go

Lines 452 to 456 in 9358c16

	collectWorker := func() {
	for {
	select {
	case collector := <-checkedCollectors:
	collector.Collect(checkedMetricChan)

will end the whole program if the collector panics, since it is run on a background goroutine. This may be an unpleasant lesson.

As a comparison, the net/http package handles panics anywhere in the serving code, although the author says they should have made this opt-in.

I note that ErrGroup added then removed this feature; reading the thread I understand that the reasons were (1) it could take a long time to reveal the error and (2) the stack trace was lost. Gather can fail immediately on panic, and can supply the stack trace in the error it returns.

[KaranPradhan266]

Hi @kakkoyun, I’d love to take this on! If it's open for external contributors.

Could you please share a bit more information on the expected behavior? Any pointers or context would be really helpful before I get started. Thanks!

[kakkoyun]

Hi @kakkoyun, I’d love to take this on! If it's open for external contributors.

Could you please share a bit more information on the expected behavior? Any pointers or context would be really helpful before I get started. Thanks!

Everything is open of external contributors. I think @bboreham did a great job on explaining the issue. I would suggest opening a draft PR and discussing any details that is not quite clear for you. We can also use https://cloud-native.slack.com/archives/C037NMVUAP3 for discussions if you think GH comments are not enough.

Just go for it!