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

proposal: runtime: support to call finalizing function when throw() is called #56702

Open
mbrancato opened this issue Nov 11, 2022 · 7 comments
Labels
Milestone

Comments

@mbrancato
Copy link

mbrancato commented Nov 11, 2022

When the runtime calls throw() for unrecoverable errors, it would be nice if there was a way to register a finalizer function to be called that would allow capturing the error in something other than stdout/stderr. This would differ from recover() in that it would still raise the error.

Background:

I'm using a web service in Go, it processes requests using goroutines. In one step, a key may be added to a map (e.g. map[string]string). In about 1 in a billion requests, under high request volume, the following fatal error is raised:

fatal error: concurrent map writes

We use an error logging library that "catches" panic() and stores the errors for tracking. I wanted to see this error be captured by that logging library. Since this error uses throw(), it cannot be handled by recover().

@gopherbot gopherbot added this to the Proposal milestone Nov 11, 2022
@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Nov 11, 2022

I don't think this is feasible. The runtime uses throw for various cases where it can't proceed. Calling general purpose Go code at that point can just makes things worse.

Would something like #42888 help your case? That proposal is accepted but not yet implemented.

@mbrancato
Copy link
Author

mbrancato commented Nov 12, 2022

It might be workable, but it seems outside the desired pattern. It would be extremely useful to be able to capture a stacktrace or other info and emit it somewhere else. The "how" here is outside my wheelhouse, but things like registering a function or interface implementation sound useful. It does seem unfortunate if the only way to catch this is to emit it to a file descriptor and let an outside, sidecar process pickup the error. Ideally, the runtime can emit it without needing an external process as that starts to complicate deployment.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Nov 12, 2022

I don't think that calling a function or method after throw is feasible. That implies running arbitrary Go code in a broken state: it will sometimes work and sometimes crash even worse with even less information. Maybe there is something we can do here, but it's not that.

@apparentlymart
Copy link

apparentlymart commented Nov 15, 2022

The use-case presented here seems to match the goals of github.com/mitchellh/panicwrap.

It achieves the goal in a pretty heavy way: it makes the program relaunch itself in a child process so that the child process can panic and the parent process can (using heuristics monitoring stderr) react to the panic by e.g. writing it to a log file. Despite the claims in the readme, it unfortunately is inevitably a bit of a leaky abstraction due to its need to intermediate stderr and watch for the panic messages. For example, it forces stderr to appear as a pipe and therefore isatty returns false even if the parent process is copying stderr data to a tty.

I mention this here both in case it might be useful to someone who has this problem today but also in case it inspires ideas about what built-in features might make this approach more robust. For example, I wonder about a mechanism to ask the runtime to send panic information to somewhere other than stderr (e.g. to some other file descriptor) so that a monitoring program like this could do its work without interfering with the standard IO handles, although I think that design would be awkward on Windows where child processes cannot inherit nonstandard filehandles.

@seankhliao
Copy link
Member

seankhliao commented Nov 15, 2022

I also don't think it's safe to execute code after a throw,
but the message the runtime generates could be nicer for monitoring systems by including clear start/end markers.

@mbrancato
Copy link
Author

mbrancato commented Nov 16, 2022

I know this isn't an easy problem to solve, so I appreciate the effort here.

With common deployments today living in containerized environments, outside of some sort of structured logging capture / parsing, gathering crash data like stack trace in a central location is extremely helpful. External processes and sidecars are not ideal. I didn't know if there could be a built-in struct type that could be defined and registered to be used when throw() is called. It could have a method that wouldn't allocate anything when called and could ship the stacktrace.

package main

import (
	"net/http"
	"net/http/crash"
)

func main() {
	crash.RegisterThrowHandler(&crash.CrashHandler{"https://crash-handler.lol/golang/crash", http.Header{"Authorization": {"Bearer Token"}}})
}

Just spitballing some ideas how something could work. The target where the stacktrace is sent could be a sidecar or a hosted error monitoring / APM service.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Nov 16, 2022

In Go it takes a runtime expert to write "a method that wouldn't allocate anything." We are not going to add any functionality to the language or standard library that can only be used safely if people are able to correctly restrict themselves to such a subset of the language. I encourage you to think of other approaches, because that one is not going to be adopted. Thanks.

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

No branches or pull requests

5 participants