BetterErrors / better_errors

Better error page for Rack apps
MIT License
6.88k stars 437 forks source link

Fix Pry `uninitialized constant` exception #395

Closed RobinDaugherty closed 7 years ago

RobinDaugherty commented 7 years ago

This fixes #393.

The fix in #329 placed some code that was specific to REPL::Pry in the ErrorPage class. This caused an exception to be raised when Pry is not enabled because REPL::Pry class is only loaded after use_pry! is called.

To fix this exception, I moved the Pry-specific behavior into REPL::Pry and added a new argument to the REPL initializer containing the most recent exception. (The code was placed in ErrorPage because that was the only place where the exception was available at the time.)

I also found that there were no specs covering REPL evaluation, so I added some. The specs failed until this refactoring was applied.

Method of loading REPL::Pry

I then made one further change: since this gem's tests run on Pry 0.9, the use of Pry 0.10+ features (specifically, the use of Pry::LastException) caused the specs to fail. I added a conditional that fixes the spec and will keep this change from affecting users of Pry 0.9. In order to effectively test the Pry REPL, I had to make changes to the way that it is loaded...

The file containing REPL::Pry can only be included when Pry is in the bundle, since it requires 'pry'; the REPL class attempts to load it only during provider selection. In order to use REPL::Pry in tests and not have it affect other specs, we must load it only when needed for an example, and then unload it after the example.

This is very hacky, and would be better refactored into a separate gem such as better_errors-pry. This other gem would be able to specify the supported Pry version in its gemspec, which is something we avoid doing in this gem because Pry is intentionally optional.