Change NotImplementedError to something else

[benhutton]

Perhaps I'm misunderstanding something, but I'd like to suggest that this gem move away from using NotImplementedError.

Take a look at the docs: http://ruby-doc.org/core-2.6/NotImplementedError.html. At issue is the OS and particular ruby implementation. You're supposed to raise that error if the current ruby or OS can't do something you need it to do. NOT if a subclass hasn't implemented an expected method.

Why is this a problem? Because NotImplementedError does not subclass StandardError, which is surprising. It subclasses ScriptError. Thus, normal rescuing (either explicitly or implicitly rescuing StandardError) won't work, and we end up rescuing things we honestly shouldn't.

Thus, I'd propose that all usage of NotImplementedError be changed to something else, or perhaps simply removed, if that would cause a semantically meaningful error to be raised (like a NoMethodError). At the very least, use something subclassed under a module, so it can be rescued specifically. And subclass that module under StandardError, so that rescuing all StandardError will work as expected, or even better, NoMethodError, since this is really a particular type of NoMethodError.

See discussion at:
http://chrisstump.online/2016/03/23/stop-abusing-notimplementederror/
https://www.reddit.com/r/ruby/comments/4cal7v/please_stop_abusing_notimplementederror/
https://airbrake.io/blog/ruby-exception-handling/notimplementederror
rubocop/ruby-style-guide#458
https://stackoverflow.com/questions/13668068/how-to-signal-not-implemented-yet

[rmosolgo]

Wow, I had no idea about that! I must have gotten confused from Python's NotImplementedError!

I think a good replacement would be something like class GraphQL::RequiredImplementationMissingError < GraphQL::Error, so that at least it would be clear that it's a library error!

[rabajaj0509]

Hello, I would like to work on this issue :)

[jbryant92]

@rahulbajaj0509 Did you get anywhere with this one? If not I can take a look.

[rmosolgo]

I don't think this was ever worked on.

[rabajaj0509]

@jbryant92 @rmosolgo hey, I am sorry I was not able to work on this one. Please feel free to take this over.

[vaihtovirta]

Hello all,
I want to pick this one unless someone has started it already.

[vaihtovirta]

Hey @rmosolgo,
can you please take a look at #2543?