5 ms·
> A quick Code Search for rows.Close() on GitHub shows that pretty much nobody is explicitly checking the return value of rows.Close() in their Go code. Seems
by nitwit005 6y ago
> A quick Code Search for rows.Close() on GitHub shows that pretty much nobody is explicitly checking the return value of rows.Close() in their Go code.
Seems to be a pattern in all languages. Plenty of C code ignoring the result of the close system calls, or Java code that just wraps exceptions from close calls in try/catch blocks that do nothing with the exception.
- PeterisP 6y agoWell, what should the code do there? At that point, the status is uncertain, there's potentially a problem but IMHO there's nothing the code can do to reliably recover from it.
- sudhirj 6y agoThe code should throw away the data it read from the connection just before it closed it, because there's a high chance of it being corrupt.
- siscia 6y agoWhat if it is a write?
- sudhirj 6y agoThat complicates things, but no different from seeing an error either way. The point is that checking for an error at point one is not enough, if you want to be sure there wasn't an error you need to check point 2 as well. In the case of a write, if you seen the success message at point 1 you might not care about a failure at point 2, because you already know the write was a success, assuming your confirmation at point 1 was explicit and well formed. In the case of read, say you read an email address, and received 'someone@somwhere.co'. The thing is, if the connection was cut you don't know if that address was supposed to be '.com', '.co.in' or '.co'. The only way you can tell in the current MySQL protocol is to check for an error on point 2 (the Close()). If you see an error, you know the read was incomplete, and you throw it away. If you don't check, you might have read the wrong (corrupt) value.
- lstodd 6y agoThat's just crazy. How do you define "just before" ?
- sudhirj 6y agoThe last iteration before the current check. But I'd assume the foolproof way would be to return an error if the Close() returns an error, erroring out the entire read. I don't know if a partial read is ever a good thing.
- Thaxll 6y agoThe code should at least log the error because it means there is something wrong somewhere, eventhough you can't do much about it it helps understand some other issues. The only way to log the error is to handle it in Go.
- apta 6y agoAnd here we see why the claim that golang forces users to handle errors is simply not true. Error handling in golang is just badly implemented.
- jlouis 6y agoNo. Error handling in Go is very explicit since it doesn't alter the control flow of the program through an exception. I'm hesitant to agree this is just a bad implementation, because you will have to make decisions on error paths in any case. If you don't do that, you are just doing bad programming. On the surface, it looks like you want to use exceptions or other control operators for handling this. But after having programmed for more than 20 years, I'm not too sure there is a good solution to this problem. Programs and operations in programs can fail. And you will have to think about these failures eventually.
- throw_m239339 6y agoThe point is that Go doesn't force any developer to deal with errors. Its error system isn't better than exceptions. In fact Go doesn't really have an error system (aside from panics), Go errors are purely a convention and no mechanism around it is actually baked into the language. It's exactly like the "good old" return codes in C. Nothing more, this isn't an error system at all. It's primitive. It's just that Go has multiple returns to make it a bit more bearable. > Programs and operations in programs can fail. Yes, and using return codes absolutely does not guarantee you will avoid panics at all. In fact you can use error codes in any other language if it pleases you, because that's not a language construct. The whole "we have exceptions but we don't have exceptions so you shouldn't use panic/recover" dance is just stupid.
- apta 6y agoWeird that you're being downvoted for stating the obvious.
- laumars 6y agoFunctions with a single return (err) are a problem but generally Go is better at forcing users to handle errors than some languages. It's also true that error handling in Go is generally less sophisticated than some languages. For developers who prefer more sophisticated tooling, Go might appear badly implemented. However that's a preference thing. I hope you can see what I'm doing here; wild generalisations just end up starting flamewars which nobody wins because everyone is using a different yardstick to measure their "correctness".
- cesarb 6y agoEven Rust, famous for caring about safety, does that: for instance, look at the documentation for File (https://doc.rust-lang.org/std/fs/struct.File.html https://doc.rust-lang.org/std/fs/struct.File.html), and you won't find any close() method, it's always automatically closed when the File goes out of scope (in its Drop implementation). Of course, this means that any errors reported by close() are simply ignored. The only way to get the errors from close() is to do a file.into_raw_fd() (which extracts the file descriptor and suppresses the Drop) followed by a FFI call to the libc close(). It gets even worse when you have a BufWriter (https://doc.rust-lang.org/std/io/struct.BufWriter.html https://doc.rust-lang.org/std/io/struct.BufWriter.html) on top, since it can have buffered data which will be written when it goes out of scope (in its Drop implementation), once again ignoring all errors; you have to remember to do a flush() (or an into_inner() to recover the File, which also does a flush()) before it goes out of scope to get any write errors (but you still won't get any errors from closing the file).
- dullgiulio 6y agoThat sentence is very misleading. Nobody checks the Close() error because the right thing to do is to call Err() like with the text.Scanner. I suspect the right github code search would reveal way less buggy code. https://golang.org/pkg/database/sql/#Rows.Err https://golang.org/pkg/database/sql/#Rows.Err