search

goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 480 forks source link

Panics: leaky abstractions #133

Closed JensRantil closed 11 years ago

JensRantil commented 11 years ago

I've just spent part of my evening reading the source code. I notice in server.go that error is returned in some cases, while panic(...) is called in other error cases. Here's an example.

A Go blog article states

The convention in the Go libraries is that even when a package uses panic internally, its external API still presents explicit error return values.

My question is, is there a specific reason as to why the raft library uses panic(...) as opposed to errors? Does it distinguish between the two somehow?

xiang90 commented 11 years ago

@JensRantil I think we use panic as assert. Feel free to send pr to clean up.

benbjohnson commented 11 years ago

@JensRantil Some of the panics are for unrecoverable errors. We should use errors instead most of the time. The ones in command.go are there because they shouldn't fail but if they do then it'll typically happen at initialization. As Xiang said, feel free to send a PR. Otherwise we can try to tackle it in the near future.

JensRantil commented 11 years ago

Ok! Thanks for quick response!