8 ms·

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup https://pkg

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated as return values, not as panics.

alexeldeib 4y ago
A goroutine created inside an http request handler (itself a goroutine) which then panics, by default will crash the whole server, not the single request. The panic could simply be an out of bounds access. That should not crash the whole server. It’s a logic bug, but you can’t “not panic”. You can trap and recover it though. Bit orthogonal to OP but relevant to your reasoning.
- bxfhjcvu 4y ago
  [dead]
- morelisp 4y ago
  > simply be an out of bounds access If you have any care for quality at all, there's nothing "simple" about your invariants being violated.
  - alexeldeib 4y ago
    So you've never written code with a bug? There are other ways to panic in go - concurrent map writes, nil pointer dereference. I'm not saying it should happen, but best practice would be a defensive posture especially when it's effectively zero cost, not hoping for the best.
    - morelisp 4y ago
      > So you've never written code with a bug? No, I write bugs all the fucking time! That's why I want my code to panic, so I know I wrote a bug. If that shit gets stashed away in a 500 response in a Prometheus metric maybe I'll chance to see it three days later. But if the container gets blown away, I'll notice.
      - pyth0 4y ago
        If your observability tools aren't firing alarms when that panic happens, you have other problems besides the panic. I would much rather find out about a bug that way vs reports from people that the service is down.
    - preseinger 4y ago
      Crashing the program _is_ the defensive posture. Panics -- concurrent map writes or nil pointer dereferences or almost anything else -- usually mean the program state has become invalid. You can't treat them like errors.
      - alexeldeib 4y ago
        Taking my own example - you can both monitor for panics per http request, while preserving SLO for other requests server by the same process. Crashing the program is strictly worse imo, since there are other concurrent requests which will now fail for no reason. I agree unhandled panicking makes sense sometimes. It’s context dependent. Program state is a good argument, but I think it really depends.
      - groestl 4y ago
        > usually mean the program state has become invalid. _some_ part of a program's state has become invalid. Probably a variable on the stack. In web servers, almost always a part that becomes very irrelevant as soon as the current request ends. In databases, the same is true at transaction boundaries. In shells it's the REPL. Exit(1) in any of those deep down the stack? No thank you, wouldn't touch that software with a 10 foot pole.
  - gpderetta 4y ago
    On a memory unsafe language if you find your invariants violated all bets are off and it is possible that the integrity of the runtime is compromised. The only safe option is to bail out to the nearest protection boundary (i.e. the process). On a memory safe language, the blast radius is potentially much smaller (in principle all objects transitively reachable from the failure point, but this is very pessimistic), so it is realistic to be able to isolate the failure in process.
    - preseinger 4y ago
      Well, memory safety is not a boolean, it's a property that's defined by the memory model of the language. That model also defines the impact of a violation.
  - deleted 4y ago
    [deleted]
jerf 4y ago
It is the way of things in an imperative language. If you catch a panic, you are also declaring to the runtime that there is nothing dangling, no locks in a bad state, etc. This is often the case. (Although since I don't think this is a well-understood aspect of what catching a panic means, it is arguably only usually true by a certain amount of coincidence.) But if you don't say that to the runtime, it can't assume it safely and terminating the program, while violent, is arguably either the best option or the only correct option. Other paradigms, like the Erlang paradigm, can have better behaviors even if a top-level evaluation fails. But in an imperative language, there really isn't anything else you should do. It is arguably one of the Clues (in the "cluestick" sense) that the imperative paradigm is perhaps not the one that should be the base of our technology. But that's a well-debated matter.
- klodolph 4y ago
  I don’t think this is really a question of whether your code is imperative, since Haskell code will terminate just as surely as Go code if you try to access an array element out of range. (Haskell’s lazy evaluation just makes it a bit harder to catch, since you need to force evaluation of the thunk within the catch statement, and it’s far too easy to end up passing your thunk to somebody who won’t catch the exception.) As a matter of Go style, of course, you should almost always defer unlock() after you lock(), but some people sometimes get clever and think that they can just lock() and unlock() manually without using defer. This is not hypothetical, and it causes other problems besides leaving dangling locks after a panic(). Somebody sticks a “return” between lock() and unlock(), without noticing, for example. So my impression of catching panic() is that it is about as safe as not catching panic(). What I mean by that is that if recover() is not safe in your code base, there is a good chance that there are other, related bugs in your code base, and being a bit more strict about using defer and not trying to be clever will go a long way.
  - morelisp 4y ago
    Since the Go 1.14 optimizations I don't believe I've found a single case where `X(); defer Unx()` has been worse. Unfortunately this was not the case before 1.14, so there's a lot of "middle-aged" code floating around setting a bad example.
camdencheek 4y ago
> The WaitGroup looks suspiciously like errgroup 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.
- morelisp 4y ago
  > This keeps panics scoped to the spawning goroutine This is exactly what's undesirable. (IMO and from my reading the GP agrees.)
- preseinger 4y ago
  > 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. 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.
  - BreakfastB0b 4y ago
    I would agree if it weren’t super easy to cause a panic in go. 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.
avita1 4y ago
This isn't catching the panic though, this is propagating the panic through the parent goroutine. The whole program will still shut down, but the stacktrace that shows up in the panic contains not only information information about the goroutine that panicked, but also the launching goroutine. That can help you figure out why the panic happened to begin with.
ilyt 4y ago
Also if that is your concern you'd just wrap the panic on your own anyway.
foobiekr 4y ago
if you've ever had to deal with a zombie JVM where an uncaught exception eventually put the whole application into a state where it wasn't, oh, hoovering the tens of thousands of temporary files it was creating because the vacuumer thread died undetected, the Go behavior is _extremely_ attractive.
vbezhenar 4y ago
I started to write Go recently and I very much prefer panics to errors. For example in web apps. Panic produces nice stack trace. Panic bubbles automatically, I can reduce amount of code significantly. Errors are awful and panics allow to write code without losing sanity. It's like good old exceptions. And if I really need it, I can catch panic and act as needed, e.g. return HTTP code or whatever.
- xyzzy_plugh 4y ago
  You should rarely reach for panic. Panics are not like exceptions, really. It's very frowned upon for panics to pass API boundaries. Use errors always, unless the state of the world is irrecoverably broken. It's difficult to come up with examples of when this may be the case. Often it's a "you're holding it wrong" kind of thing. For example, a common idiom is wrapping something like func Parse(s string) (Foo, error) to be usable in contexts where error handling isn't possible, such as variable initialization. func Must(f Foo, err error) Foo { if err != nil { panic(err) } return f } which could be used at the global level like so: var someFoo = Must(Parse("hey")) This is acceptable because this code is static and will either panic at boot always or never.
  - vbezhenar 4y ago
    Errors don't have stack traces so they're unusable. And I see nothing wrong with handling panics. They're the same exceptions. I got SQL exception, I panic, handler catches panic, logs its stacktrace and returns HTTP 500. Awesome and no boring error handling.
    - bit_flipper 4y ago
      It's fine to dislike Go's philosophy on error handling, but in order to save you and your co-workers a lot of headache down the road, I'd recommend you just use another language. This is not something you want to do in Go and very few folks who program in Go would be happy to work with that style of code. In case you actually are interested in Go's take on stack traces: You are intended to annotate your errors so you can build your own stack traces with whatever information you want. This leads to better error messages because additional context (values of things) can be logged with your custom stack trace.
Smurfix 4y ago
How the heck should a library author know whether an error is recoverable or not? In many situations that depends entirely on the caller. Not-so-contrived example: a config options contains a value that leads to a division by zero in some library I'm using. I don't want that to crash my whole program, I want to tell the user "hey this part didn't work" but be able to continue with the other parts. Without writing a ton of boilerplate code.

alexeldeib 4y ago

A goroutine created inside an http request handler (itself a goroutine) which then panics, by default will crash the whole server, not the single request. The panic could simply be an out of bounds access. That should not crash the whole server. It’s a logic bug, but you can’t “not panic”. You can trap and recover it though. Bit orthogonal to OP but relevant to your reasoning.

bxfhjcvu 4y ago
[dead]
morelisp 4y ago
> simply be an out of bounds access If you have any care for quality at all, there's nothing "simple" about your invariants being violated.
- alexeldeib 4y ago
  So you've never written code with a bug? There are other ways to panic in go - concurrent map writes, nil pointer dereference. I'm not saying it should happen, but best practice would be a defensive posture especially when it's effectively zero cost, not hoping for the best.
  - morelisp 4y ago
    > So you've never written code with a bug? No, I write bugs all the fucking time! That's why I want my code to panic, so I know I wrote a bug. If that shit gets stashed away in a 500 response in a Prometheus metric maybe I'll chance to see it three days later. But if the container gets blown away, I'll notice.
    - pyth0 4y ago
      If your observability tools aren't firing alarms when that panic happens, you have other problems besides the panic. I would much rather find out about a bug that way vs reports from people that the service is down.
  - preseinger 4y ago
    Crashing the program _is_ the defensive posture. Panics -- concurrent map writes or nil pointer dereferences or almost anything else -- usually mean the program state has become invalid. You can't treat them like errors.
    - alexeldeib 4y ago
      Taking my own example - you can both monitor for panics per http request, while preserving SLO for other requests server by the same process. Crashing the program is strictly worse imo, since there are other concurrent requests which will now fail for no reason. I agree unhandled panicking makes sense sometimes. It’s context dependent. Program state is a good argument, but I think it really depends.
    - groestl 4y ago
      > usually mean the program state has become invalid. _some_ part of a program's state has become invalid. Probably a variable on the stack. In web servers, almost always a part that becomes very irrelevant as soon as the current request ends. In databases, the same is true at transaction boundaries. In shells it's the REPL. Exit(1) in any of those deep down the stack? No thank you, wouldn't touch that software with a 10 foot pole.
- gpderetta 4y ago
  On a memory unsafe language if you find your invariants violated all bets are off and it is possible that the integrity of the runtime is compromised. The only safe option is to bail out to the nearest protection boundary (i.e. the process). On a memory safe language, the blast radius is potentially much smaller (in principle all objects transitively reachable from the failure point, but this is very pessimistic), so it is realistic to be able to isolate the failure in process.
  - preseinger 4y ago
    Well, memory safety is not a boolean, it's a property that's defined by the memory model of the language. That model also defines the impact of a violation.
- deleted 4y ago
  [deleted]

jerf 4y ago

It is the way of things in an imperative language. If you catch a panic, you are also declaring to the runtime that there is nothing dangling, no locks in a bad state, etc. This is often the case. (Although since I don't think this is a well-understood aspect of what catching a panic means, it is arguably only usually true by a certain amount of coincidence.) But if you don't say that to the runtime, it can't assume it safely and terminating the program, while violent, is arguably either the best option or the only correct option. Other paradigms, like the Erlang paradigm, can have better behaviors even if a top-level evaluation fails. But in an imperative language, there really isn't anything else you should do. It is arguably one of the Clues (in the "cluestick" sense) that the imperative paradigm is perhaps not the one that should be the base of our technology. But that's a well-debated matter.

klodolph 4y ago
I don’t think this is really a question of whether your code is imperative, since Haskell code will terminate just as surely as Go code if you try to access an array element out of range. (Haskell’s lazy evaluation just makes it a bit harder to catch, since you need to force evaluation of the thunk within the catch statement, and it’s far too easy to end up passing your thunk to somebody who won’t catch the exception.) As a matter of Go style, of course, you should almost always defer unlock() after you lock(), but some people sometimes get clever and think that they can just lock() and unlock() manually without using defer. This is not hypothetical, and it causes other problems besides leaving dangling locks after a panic(). Somebody sticks a “return” between lock() and unlock(), without noticing, for example. So my impression of catching panic() is that it is about as safe as not catching panic(). What I mean by that is that if recover() is not safe in your code base, there is a good chance that there are other, related bugs in your code base, and being a bit more strict about using defer and not trying to be clever will go a long way.
- morelisp 4y ago
  Since the Go 1.14 optimizations I don't believe I've found a single case where `X(); defer Unx()` has been worse. Unfortunately this was not the case before 1.14, so there's a lot of "middle-aged" code floating around setting a bad example.

camdencheek 4y ago

> The WaitGroup looks suspiciously like errgroup 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.

morelisp 4y ago
> This keeps panics scoped to the spawning goroutine This is exactly what's undesirable. (IMO and from my reading the GP agrees.)
preseinger 4y ago
> 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. 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.
- BreakfastB0b 4y ago
  I would agree if it weren’t super easy to cause a panic in go. 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.

avita1 4y ago

This isn't catching the panic though, this is propagating the panic through the parent goroutine. The whole program will still shut down, but the stacktrace that shows up in the panic contains not only information information about the goroutine that panicked, but also the launching goroutine. That can help you figure out why the panic happened to begin with.

ilyt 4y ago

Also if that is your concern you'd just wrap the panic on your own anyway.

foobiekr 4y ago

if you've ever had to deal with a zombie JVM where an uncaught exception eventually put the whole application into a state where it wasn't, oh, hoovering the tens of thousands of temporary files it was creating because the vacuumer thread died undetected, the Go behavior is _extremely_ attractive.

vbezhenar 4y ago

I started to write Go recently and I very much prefer panics to errors. For example in web apps. Panic produces nice stack trace. Panic bubbles automatically, I can reduce amount of code significantly. Errors are awful and panics allow to write code without losing sanity. It's like good old exceptions. And if I really need it, I can catch panic and act as needed, e.g. return HTTP code or whatever.

xyzzy_plugh 4y ago
You should rarely reach for panic. Panics are not like exceptions, really. It's very frowned upon for panics to pass API boundaries. Use errors always, unless the state of the world is irrecoverably broken. It's difficult to come up with examples of when this may be the case. Often it's a "you're holding it wrong" kind of thing. For example, a common idiom is wrapping something like func Parse(s string) (Foo, error) to be usable in contexts where error handling isn't possible, such as variable initialization. func Must(f Foo, err error) Foo { if err != nil { panic(err) } return f } which could be used at the global level like so: var someFoo = Must(Parse("hey")) This is acceptable because this code is static and will either panic at boot always or never.
- vbezhenar 4y ago
  Errors don't have stack traces so they're unusable. And I see nothing wrong with handling panics. They're the same exceptions. I got SQL exception, I panic, handler catches panic, logs its stacktrace and returns HTTP 500. Awesome and no boring error handling.
  - bit_flipper 4y ago
    It's fine to dislike Go's philosophy on error handling, but in order to save you and your co-workers a lot of headache down the road, I'd recommend you just use another language. This is not something you want to do in Go and very few folks who program in Go would be happy to work with that style of code. In case you actually are interested in Go's take on stack traces: You are intended to annotate your errors so you can build your own stack traces with whatever information you want. This leads to better error messages because additional context (values of things) can be logged with your custom stack trace.

Smurfix 4y ago

How the heck should a library author know whether an error is recoverable or not? In many situations that depends entirely on the caller. Not-so-contrived example: a config options contains a value that leads to a division by zero in some library I'm using. I don't want that to crash my whole program, I want to tell the user "hey this part didn't work" but be able to continue with the other parts. Without writing a ton of boilerplate code.