| ▲ | hambes 5 days ago |
| Solution to the specifically mentioned problem: Don't use string-based errors, use sentinel errors [1]. More generally: Don't produce code where consumers of your API are the least bit inclined to rely on non-technical strings. Instead use first-level language constructs like predefined error values, types or even constants that contain the non-technical string so that API consumers can compare the return value againnst the constant instead of hard-coding the contained string themselves. Hyrum's Law is definitely a thing, but its effects can be mitigated. [1]: https://thomas-guettler.de/go/wrapping-and-sentinel-errors |
|
| ▲ | gwd 5 days ago | parent | next [-] |
| The frustrating thing is that the error in question already is a sentinel error -- Grafana (the top-level culprit in the linked search) should be using `errors.As(&http.MaxBytesError{})` rather than doing a string compare. The whole point of Hyrum's Law is that it doesn't matter how well you design your API: no matter what, people will depend on its behavior rather than its contract. |
| |
| ▲ | sssddfffdssasdf 5 days ago | parent | next [-] | | But it looks like that until 3 years ago, this string comparison was the only way to do it. https://github.com/golang/go/pull/49359/files | | |
| ▲ | gwd 5 days ago | parent | next [-] | | Good catch. So in a sense this isn't really Hyrum's Law (which would be more appropriate to things like the Sim City / Windows 3.x UAF bug described in a sibling comment); it's more like, if people need to do something, and you don't give people an explicit way to do it, they'll find an implicit way, and then you're stuck supporting whatever that happened to be. | | |
| ▲ | ekidd 5 days ago | parent [-] | | There was a well-known trick in MacOS development in the 90s. You couldn't always avoid relying on undocumented behavior. The docs were incomplete and occasionally vague. What you could do was try to rely on the same undocumented behavior as everyone else. This way, if Apple broke you, they'd break half their ecosystem at the same time. |
| |
| ▲ | lokar 5 days ago | parent | prev [-] | | Or they could have fixed the error (adding the type) instead of matching the string. |
| |
| ▲ | LudwigNagasena 5 days ago | parent | prev [-] | | Early Go lacked lots of features such as errors.As. It was and still is sometimes idiomatic to generate Go because it is so featureless and writing it is often a chore. So it is very much about how well you design your API. |
|
|
| ▲ | Svip 5 days ago | parent | prev | next [-] |
| In your example, the onus is on the consumer not the provider. I could still be writing code that checks if `err.String() == "no more tea available."`. I agree, I shouldn't do that, but nothing is preventing me from doing that. Additionally, errors.Is is a relatively recent addition to Go, so by the time people would check for errors like this, it was just easier to check the literal string. But as an API provider in Go, you cannot prevent your consumers from checking the return values of .String(). |
| |
| ▲ | hambes 5 days ago | parent [-] | | Unfortunately true. The Go maintainers might not agree with me on this, but I think in this case consumers have to learn the hard way. Go tries to always be backwards compatible, but I don't think that trying to be backwards compatible with incorrect usage is ever the right choice. | | |
| ▲ | LudwigNagasena 5 days ago | parent [-] | | So the people who decided to make a stringly type error with `errors.New("http: request body too large")` and make you suffer, now can remove a stringly typed error and make you suffer even more? What would the lesson be? What would consumers learn? | | |
| ▲ | hambes 5 days ago | parent | next [-] | | I don't understand your point. The lesson is "don't rely on magic strings, instead rely on exported and documented constants, otherwise your code might break". | | |
| ▲ | LudwigNagasena 5 days ago | parent [-] | | My point is that a few years ago there was no exported and document constant. The lesson should be "provide sensible tools, otherwise your consumers will have to rely on implementation details for the most basic expected stuff". | | |
| ▲ | stonemetal12 5 days ago | parent | next [-] | | >My point is that a few years ago there was no exported and document constant. Then the feature didn't exist. Figuring out undocumented implementation details to "make it work" is asking for it to be broken in the future. So if you are unwilling or unable to support fixing it in the future then don't do that. If it is "the most basic expected stuff" then quite literally make the determination that it isn't ready for use. A lot of Go was and maybe still be half baked and not ready for production. It is ok to recognize that and not use it. | | |
| ▲ | Joker_vD 4 days ago | parent [-] | | I am glad that your circumstances are such that you can just stop working on a project when the tooling it uses turns out to be inadequate, wait five years, and then come back when it improves. Unfortunately, many people can't really do that: when the ecosystem turns out to be somewhat inadequate in a project that's already been in use for couple of years, their options are either "just make it work one way or another, who cares if it's a hardcoded string, we have to ship the fix ASAP" or "rewrite it all in Rust/X, allegedly their ecosystem is production-ready". | | |
| ▲ | outworlder 4 days ago | parent | next [-] | | > I am glad that your circumstances are such that you can just stop working on a project when the tooling it uses turns out to be inadequate, wait five years, and then come back when it improves. Is it that terrible to just handle an error as an error, without having to know exactly what the error was? If you see some of the codebases which rely on the error, they are trying to be too clever and doing things like returning a 400 instead of 500 if that's the specific error message returned. Is that really necessary? Unless the codebase can take corrective actions (and it could still attempt to do it regardless if that's the case), there's really no point trying to be cute. An error is returned, and that's that. | |
| ▲ | dwattttt 4 days ago | parent | prev [-] | | > "just make it work one way or another, who cares if it's a hardcoded string, we have to ship the fix ASAP" Sure, but now that there's a "correct" way to do this, you don't get to complain that the hacky thing you did needs to keep being supported. You fix the hacky thing you did, or you make peace that you're still doing the hacky thing, problems it causes and all. | | |
| ▲ | beautron 4 days ago | parent [-] | | I love that the Go project takes compatibility so seriously. And I think taking Hyrum's Law into account is necessary, if what you're serious about is compatibility itself. Being serious about compatibility allows the concept of a piece of software being finished. If I finished writing a book twelve years ago, you could still read it today. But if I finished writing a piece of software twelve years ago, could you still build and run it today? Without having to fix anything? Without having to fix lots of things? > Sure, but now that there's a "correct" way to do this, you don't get to complain that the hacky thing you did needs to keep being supported. But that's the whole point and beauty of Go's compatibility promise. Once you finish getting something working, you finished getting it working. It works. What I don't want, is for my programming platform to suddenly say that the way I got the thing working is no longer supported. I am no longer finished getting it working. I will never be finished getting it working. Go is proving that a world with permanently working software is possible (vs a world with software that breaks over time). |
|
|
| |
| ▲ | 5 days ago | parent | prev | next [-] | | [deleted] | |
| ▲ | estebarb 4 days ago | parent | prev [-] | | That is the kind of stuff I would have expected `go vet` to fix. |
|
| |
| ▲ | 4 days ago | parent | prev [-] | | [deleted] |
|
|
|
|
| ▲ | karel-3d 5 days ago | parent | prev | next [-] |
| Using string error comparisons was the only way to do this few years ago; and Go has a backwards compatibility promise. |
| |
|
| ▲ | cedws 5 days ago | parent | prev | next [-] |
| Code that checks raw error strings is just plain bad and should be exempt from Go’s backwards compatibility guarantees. There is almost never an excuse for it, especially in stdlib. |
|
| ▲ | pjmlp 4 days ago | parent | prev | next [-] |
| Go original design is to blame, for a long time string based errors were the only way, some standard library packages still have them if I am not mistaken, let alone the whole ecosystem. That is what happens when history of programming languages is ignored on purpose, followed by a "design as we go" approach. |
|
| ▲ | 5 days ago | parent | prev | next [-] |
| [deleted] |
|
| ▲ | adontz 5 days ago | parent | prev [-] |
| Honestly, this is so much worse than "catch". It's what a "catch" would look like in "C". |
| |
| ▲ | hambes 5 days ago | parent | next [-] | | It might look worse than catch, but it's much more predictable and less goto-y. | | |
| ▲ | guappa 5 days ago | parent | next [-] | | goto was only bad when used to save code and jump indiscriminately. To handle errors is no problem at all. | | |
| ▲ | froh 5 days ago | parent | next [-] | | yes, yes, yes! see the Linux Kernel for plenty of such good and readable uses of go-to, considered useful: "on error, jump there in the cleanup sequence ..." | |
| ▲ | _flux 5 days ago | parent | prev [-] | | ..as long as you don't make mistakes. I fixed enough goto bugs in Xorg when I was fixing Coverity-issues in Xorg that I can see the downsides of this easy way of error handling. | | |
| ▲ | guappa 4 days ago | parent [-] | | We're comparing to go here, not with a language with proper error handling. |
|
| |
| ▲ | int_19h 4 days ago | parent | prev [-] | | If "catch" is goto-y (and it kinda is), then so is "defer". |
| |
| ▲ | kbolino 5 days ago | parent | prev [-] | | The biggest difference between try-catch and error values syntactically IMO is that the former allows you to handle a specific type of error from an unspecified place and the latter allows you to handle an unspecified type of error from a specific place. So the type checking is more cumbersome with error values whereas enclosing every individual source of exceptions in its own try-catch block is more cumbersome than error values. You usually don't do that, but you usually don't type-check error values either. |
|
