3 ms·

> approximately 3/4 of the significant lines of code are endless repetitions of: if err { return nil, err } 3/4? How are you managing to have so many cases of

> approximately 3/4 of the significant lines of code are endless repetitions of: if err { return nil, err } 3/4? How are you managing to have so many cases of blindly passing an error up the stack without introducing problematic coupling? I would suggest that if you are able to realistically do this more than a couple of times total in an application, you have introduced way too much pointless indirection and should take a closer look at your overall design. Something is amiss. And that goes for any language – not something exclusive to Go. Blindly propagating an error up the stack using exception handlers, for example, is prone to the same problematic coupling and indicates the same design problems if seen in more than rare situations.

dd82 3y ago
its of course hyperbolic but frequent enough in any substantial codebase to be a wart on the language.
- randomdata 3y ago
  Most codebases would not justify the use of 'return nil, err' at all. The language certainly isn't going to stop you, but you are going to make life miserable for future developers if you do so. There are rare circumstances where it is the right thing, but if you are seeing more than one or two instances in a substantial codebase, something isn't right. If it is a common occurrence, and you are not purposefully trying to demonstrate your hatred of future developers, you've no doubt introduced way too much unnecessary complexity – which too is going to make life miserable for future developers.
  - dd82 3y ago
    would love to see some examples you cite. This pattern is prevalent in all golang I've seen, whether in libs or work. And I've only been working in the language full time for three months.
    - randomdata 3y ago
      I would love to have the original question answered. Perhaps you (or the people who have contributed the code of which you speak) have stumbled upon some solution to the coupling problem, in which case I would then understand the choice. Frankly, the entire Go community is no doubt keen to hear it. The 'try' proposal fell apart because nobody could figure out a good solution to that problem at the time, and could not find justification for a whole new feature for rare occasions. If `if err != nil { return nil, err }` were to actually become tenable in most cases then said proposal could be revived based on your information. It was otherwise well received.
      - bvrmn 3y ago
        It would be wonderful to see your non-coupled design without middle layers knowing about top context to handle errors without bubbling it up. It already sounds like a failure.
        randomdata 3y ago
        I must misunderstand, but your request does, indeed, sound like a failure – and not at all related to the discussion taking place. Perhaps you could explain how "middle layers" and "top context" fit?
        closeparen 3y ago
        "Handling" an error, without propagating it to the caller, can only mean deciding that the thing attempted didn't really need to happen. This situation is extremely rare: if it's appropriate to swallow an error, you may as well not even attempt the thing. We don't normally write "opportunistic" functionality into our software. But when we do, it's the jurisdiction of the caller to say that the it doesn't matter if the called function fails. It's almost never appropriate for the callee to decide to eat its own failures.
closeparen 3y ago
For a gRPC/HTTP request that calls another microservice and writes to a database, you've got: * Handler maps the wire format message to an internal entity. Return a bad-request type error if it fails. * Handler calls controller. Perhaps some specific error conditions get dedicated status codes, the rest get 5XX. * Controller calls gateway. Except in rare cases where the external call is optional, you probably just bubble this to the handler. * Gateway maps internal entity to wire protocol format request. Sometimes this transformation can have errors; bubble these up. * Gateway calls wire protocol client. Wire protocol client may have built-in retries, or gateway may have application-level retries. In any case, if retries are exhausted, bubble this up to the controller. * Gateway maps wire protocol response to internal entity. Depending on the schema, this can often fail to be well-formed, even if the request is "successful." These errors also need reported to the controller. * If everything is successful up to this point, controller calls repository. Bubble errors to handler. * Repository maps internal entity to storage model. Sometimes this transformation is also fallible. * Repository calls storage client. These errors might be retryable but after retries, need bubbling up. * Finally, storage client returns successful value to repository returns to controller returns to handler returns to wire protocol server. This is just a hello-world level microservice. We have thousands of them, with easily a dozen endpoints each and probably 5+ interactions per endpoint on average. And oh yeah, every single one of these error return sites needs a unit test case.
anon-3988 3y ago
Mate, the code in question also simply propagates error up the stack. I have seen this all the time in pretty much all program. There's only a few times where exceptions are actually handled; most of the time, they are truly exceptional. What is anyone going to do when printf/malloc/read/open/close fails? If I am using a library, I pretty much expect each and every one of its API to return a Result<T> or Option<T>. Those that don't either: do something funny to hide the bad states or simply crash and die which means I have to do some double checks on my end.
- IcePic 3y ago
  > What is anyone going to do when printf/malloc/read/open/close fails? As a sysadmin, I would love for the code to log/print the "#€%"#€%#€ filename when open() or read() fails, and not just bubble up some generic "something went wrong, fix something" and have me dive into strace/truss/ktrace just to know that /home/foo/badperms.txt could not be opened. For some reason, all these wrapper libraries and frameworks and stuff are super good at hiding things for which we used to get decent errors, like "could not open tcp port 443" or "file: ./badperms.txt open() failed" but as the layers stacked on top of eachother more and more, the code calling "set-up-totaly-secure-sending-of-file-to-remote-http-endpoint-and-renew-LE-cert-if-needed()" has so many moving parts that the program can only say "worked perfectly" or "dang, noone in the world knows what went wrong, try again tomorrow, worked on my laptop once before deploy". So while it is not "fun" to handle all these particular errors one by one, when we stop fussing about details, we make someones life miserable as the filesystem goes full/quota, or when networks/firewalls hinder traffic if we can't even tell the user which of those two occured because it would be "tedious" to pay attention to so much detail when all I wanted was my program to be short and sweet.
thayne 3y ago
In my experience there are usually only a few places in an application that actually care about errors. You typically have something near the top level that reports errors to the user or client, whether that is printing to stderr, showing a dialog, or returning an error response over the network. You might have something that logs errors. That might be in the same place you report to the user, or it might happen where errors originate. At some boundaries, especially between a library and the calling application it might wrap the original error in a different error. If you have retries, that is probably handled by a framework that in turn returns errors if the retries fail. And maybe in some rare cases you can gracefully degrade or try an alternative in the case of an error. Everywhere else, you probably just want to propagate it up the top level error reporting facility. I don't think you should blindly pass up all errors, but IME propagating upwards is usually the right thing to do in your middle layers.
- deleted 3y ago
  [deleted]

dd82 3y ago

its of course hyperbolic but frequent enough in any substantial codebase to be a wart on the language.

randomdata 3y ago
Most codebases would not justify the use of 'return nil, err' at all. The language certainly isn't going to stop you, but you are going to make life miserable for future developers if you do so. There are rare circumstances where it is the right thing, but if you are seeing more than one or two instances in a substantial codebase, something isn't right. If it is a common occurrence, and you are not purposefully trying to demonstrate your hatred of future developers, you've no doubt introduced way too much unnecessary complexity – which too is going to make life miserable for future developers.
- dd82 3y ago
  would love to see some examples you cite. This pattern is prevalent in all golang I've seen, whether in libs or work. And I've only been working in the language full time for three months.
  - randomdata 3y ago
    I would love to have the original question answered. Perhaps you (or the people who have contributed the code of which you speak) have stumbled upon some solution to the coupling problem, in which case I would then understand the choice. Frankly, the entire Go community is no doubt keen to hear it. The 'try' proposal fell apart because nobody could figure out a good solution to that problem at the time, and could not find justification for a whole new feature for rare occasions. If `if err != nil { return nil, err }` were to actually become tenable in most cases then said proposal could be revived based on your information. It was otherwise well received.
    - bvrmn 3y ago
      It would be wonderful to see your non-coupled design without middle layers knowing about top context to handle errors without bubbling it up. It already sounds like a failure.
      - randomdata 3y ago
        I must misunderstand, but your request does, indeed, sound like a failure – and not at all related to the discussion taking place. Perhaps you could explain how "middle layers" and "top context" fit?
        closeparen 3y ago
        "Handling" an error, without propagating it to the caller, can only mean deciding that the thing attempted didn't really need to happen. This situation is extremely rare: if it's appropriate to swallow an error, you may as well not even attempt the thing. We don't normally write "opportunistic" functionality into our software. But when we do, it's the jurisdiction of the caller to say that the it doesn't matter if the called function fails. It's almost never appropriate for the callee to decide to eat its own failures.

closeparen 3y ago

For a gRPC/HTTP request that calls another microservice and writes to a database, you've got: * Handler maps the wire format message to an internal entity. Return a bad-request type error if it fails. * Handler calls controller. Perhaps some specific error conditions get dedicated status codes, the rest get 5XX. * Controller calls gateway. Except in rare cases where the external call is optional, you probably just bubble this to the handler. * Gateway maps internal entity to wire protocol format request. Sometimes this transformation can have errors; bubble these up. * Gateway calls wire protocol client. Wire protocol client may have built-in retries, or gateway may have application-level retries. In any case, if retries are exhausted, bubble this up to the controller. * Gateway maps wire protocol response to internal entity. Depending on the schema, this can often fail to be well-formed, even if the request is "successful." These errors also need reported to the controller. * If everything is successful up to this point, controller calls repository. Bubble errors to handler. * Repository maps internal entity to storage model. Sometimes this transformation is also fallible. * Repository calls storage client. These errors might be retryable but after retries, need bubbling up. * Finally, storage client returns successful value to repository returns to controller returns to handler returns to wire protocol server. This is just a hello-world level microservice. We have thousands of them, with easily a dozen endpoints each and probably 5+ interactions per endpoint on average. And oh yeah, every single one of these error return sites needs a unit test case.

anon-3988 3y ago

Mate, the code in question also simply propagates error up the stack. I have seen this all the time in pretty much all program. There's only a few times where exceptions are actually handled; most of the time, they are truly exceptional. What is anyone going to do when printf/malloc/read/open/close fails? If I am using a library, I pretty much expect each and every one of its API to return a Result<T> or Option<T>. Those that don't either: do something funny to hide the bad states or simply crash and die which means I have to do some double checks on my end.

IcePic 3y ago
> What is anyone going to do when printf/malloc/read/open/close fails? As a sysadmin, I would love for the code to log/print the "#€%"#€%#€ filename when open() or read() fails, and not just bubble up some generic "something went wrong, fix something" and have me dive into strace/truss/ktrace just to know that /home/foo/badperms.txt could not be opened. For some reason, all these wrapper libraries and frameworks and stuff are super good at hiding things for which we used to get decent errors, like "could not open tcp port 443" or "file: ./badperms.txt open() failed" but as the layers stacked on top of eachother more and more, the code calling "set-up-totaly-secure-sending-of-file-to-remote-http-endpoint-and-renew-LE-cert-if-needed()" has so many moving parts that the program can only say "worked perfectly" or "dang, noone in the world knows what went wrong, try again tomorrow, worked on my laptop once before deploy". So while it is not "fun" to handle all these particular errors one by one, when we stop fussing about details, we make someones life miserable as the filesystem goes full/quota, or when networks/firewalls hinder traffic if we can't even tell the user which of those two occured because it would be "tedious" to pay attention to so much detail when all I wanted was my program to be short and sweet.

thayne 3y ago

In my experience there are usually only a few places in an application that actually care about errors. You typically have something near the top level that reports errors to the user or client, whether that is printing to stderr, showing a dialog, or returning an error response over the network. You might have something that logs errors. That might be in the same place you report to the user, or it might happen where errors originate. At some boundaries, especially between a library and the calling application it might wrap the original error in a different error. If you have retries, that is probably handled by a framework that in turn returns errors if the retries fail. And maybe in some rare cases you can gracefully degrade or try an alternative in the case of an error. Everywhere else, you probably just want to propagate it up the top level error reporting facility. I don't think you should blindly pass up all errors, but IME propagating upwards is usually the right thing to do in your middle layers.

deleted 3y ago
[deleted]