(no title)
camdencheek | 3 years ago
I heavily used errgroup before creating conc, so the design is likely strongly influenced by that of errgroup even if not consciously. Conc was partially built to address the shortcomings of errgroup (from my perspective). Probably worth adding a "prior art" section to the README, but many of the ideas in conc are not unique.
> In go land, this seems desirable.
I mostly agree, which is why `Wait()` propagates the panic rather than returning it or logging it. This keeps panics scoped to the spawning goroutine and enables getting stacktraces from both the spawning goroutine and the spawned goroutine, which is quite useful for debugging.
That said, crashing the whole webserver because of one misbehaving request is not necessarily a good tradeoff. Conc moves panics into the spawning goroutine, which makes it possible to do things like catch panics at the top of a request and return a useful error to the caller, even if that error is just "nil pointer dereference" with a stacktrace. It's up to the user to decide what to do with propagated panics.
bertmuthalaly|3 years ago
preseinger|3 years ago
The problem is that panics aren't "goroutine scoped" in terms of their potential impact. So it really shouldn't be up to the user to decide how to handle a panic. Application code shouldn't handle panics at all! They're not just a different way to yield an error, they're critical bugs which shouldn't occur at all.
camdencheek|3 years ago
I'm with ya there. However, there are also many classes of logic errors that are not goroutine-scoped. And there are many panics that do not have impact outside of the goroutine's scope. In my experience, this is true of most panics.
In practice, panics happen. They are (almost) always indicative of a bug, and almost always mean there is something that needs fixed. However, if a subsystem of my application is broken and panicking, there's a pretty good chance that reporting the panic without crashing the process will provide a better end user experience than just blowing up.
Yes, that means I'm accepting the risk that my application is left in an inconsistent state, but coupled with good observability/reporting, that's a tradeoff I'm willing to make.
(bonus: this is especially true when propagating panics allow me to capture more debugging information to fix the panics faster)
BreakfastB0b|3 years ago
Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic.
I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potentially modified the application state in an unknown way.
It’s like not in C, or C++ where out of bounds access has now put the entire application into an unknown state.
jasonhansel|3 years ago
This would be fine for a low-level language like C where you need to allow SEGFAULTs, but designing it into a high-level language makes no sense.
morelisp|3 years ago
This is exactly what's undesirable. (IMO and from my reading the GP agrees.)