diff --git a/.coveralls.yml b/.coveralls.yml new file mode 100644 index 00000000..91600595 --- /dev/null +++ b/.coveralls.yml @@ -0,0 +1 @@ +service_name: travis-ci diff --git a/.gitignore b/.gitignore index 88654958..d3f6a852 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ /doc /.bundle /vendor/gems + +/gemfiles/*.gemfile.lock +/gemfiles/.bundle diff --git a/.travis.yml b/.travis.yml index f96813da..0193b6d9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,111 @@ -sudo: false language: ruby cache: bundler +notifications: + webhooks: + # With COVERALLS_PARALLEL, coverage information sent to coveralls will not be processed until + # this webhook is sent. + # https://coveralls.zendesk.com/hc/en-us/articles/203484329-Parallel-Build-Webhook + - secure: "YnHYbTq51ySistjvOxsuNhyg4GLuUffEJstTYeGYXiBF7HG5h43IVYo8KNuLzwkgsOYBcNo+YMdQX7qCqJffSbhsr1FZRSzBmjFFxcyD4hu+ukM2theZ4mePVAZiePscYvQPRNY4hIb4d3egStJEytkalDhB3sOebF57tIaCssg=" rvm: - - 2.0 - - 2.1 - - 2.2 + - 2.2.10 + - 2.3.8 + - 2.4.9 + - 2.5.7 + - 2.6.5 + - 2.7.0 + - ruby-head + - truffleruby-head +gemfile: + - gemfiles/rails42.gemfile + - gemfiles/rails50.gemfile + - gemfiles/rails51.gemfile + - gemfiles/rails52.gemfile + - gemfiles/rails60.gemfile + - gemfiles/rails42_haml.gemfile + - gemfiles/rails50_haml.gemfile + - gemfiles/rails51_haml.gemfile + - gemfiles/rails52_haml.gemfile + - gemfiles/rails60_haml.gemfile + - gemfiles/rails42_boc.gemfile + - gemfiles/rails50_boc.gemfile + - gemfiles/rails51_boc.gemfile + - gemfiles/rails52_boc.gemfile + - gemfiles/rails60_boc.gemfile + - gemfiles/rack.gemfile + - gemfiles/rack_boc.gemfile + - gemfiles/pry09.gemfile + - gemfiles/pry010.gemfile + - gemfiles/pry011.gemfile +matrix: + fast_finish: true + allow_failures: + - rvm: ruby-head + - gemfile: gemfiles/pry010.gemfile + - gemfile: gemfiles/pry011.gemfile + exclude: + - rvm: 2.2.10 + gemfile: gemfiles/rails60.gemfile + - rvm: 2.2.10 + gemfile: gemfiles/rails60_boc.gemfile + - rvm: 2.2.10 + gemfile: gemfiles/rails60_haml.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails42.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails42_boc.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails42_haml.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails60.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails60_boc.gemfile + - rvm: 2.3.8 + gemfile: gemfiles/rails60_haml.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails42.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails42_boc.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails42_haml.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails60.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails60_boc.gemfile + - rvm: 2.4.9 + gemfile: gemfiles/rails60_haml.gemfile + - rvm: 2.5.7 + gemfile: gemfiles/rails42.gemfile + - rvm: 2.5.7 + gemfile: gemfiles/rails42_boc.gemfile + - rvm: 2.5.7 + gemfile: gemfiles/rails42_haml.gemfile + - rvm: 2.6.5 + gemfile: gemfiles/rails42.gemfile + - rvm: 2.6.5 + gemfile: gemfiles/rails42_boc.gemfile + - rvm: 2.6.5 + gemfile: gemfiles/rails42_haml.gemfile + - rvm: 2.7.0 + gemfile: gemfiles/rails42.gemfile + - rvm: 2.7.0 + gemfile: gemfiles/rails42_boc.gemfile + - rvm: 2.7.0 + gemfile: gemfiles/rails42_haml.gemfile + - rvm: ruby-head + gemfile: gemfiles/rails42.gemfile + - rvm: ruby-head + gemfile: gemfiles/rails42_boc.gemfile + - rvm: ruby-head + gemfile: gemfiles/rails42_haml.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rails42_boc.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rails50_boc.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rails51_boc.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rails52_boc.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rails60_boc.gemfile + - rvm: truffleruby-head + gemfile: gemfiles/rack_boc.gemfile diff --git a/CHANGELOG.md b/CHANGELOG.md index 00fc2466..f5f26803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,3 @@ # Changelog -See https://github.com/charliesome/better_errors/releases +See https://github.com/BetterErrors/better_errors/releases diff --git a/Gemfile b/Gemfile index 89418348..07cd2076 100644 --- a/Gemfile +++ b/Gemfile @@ -2,10 +2,5 @@ source 'https://rubygems.org' gemspec -gem "rake" -gem "rack" -gem "rspec", "2.14.1" -gem "binding_of_caller", platforms: :ruby -gem "pry", "0.9.12" -gem "yard" -gem "kramdown" +# gem "pry-byebug" +gem 'simplecov', require: false diff --git a/LICENSE.txt b/LICENSE.txt index 7de0d08b..59342cca 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -1,4 +1,4 @@ -Copyright (c) 2012-2015 Charlie Somerville +Copyright (c) 2012-2016 Charlie Somerville MIT License diff --git a/README.md b/README.md index d1901aa9..d428c13d 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,24 @@ -# Better Errors [![Gem Version](https://img.shields.io/gem/v/better_errors.svg)](https://rubygems.org/gems/better_errors) [![Build Status](https://travis-ci.org/charliesome/better_errors.svg)](https://travis-ci.org/charliesome/better_errors) [![Code Climate](https://img.shields.io/codeclimate/github/charliesome/better_errors.svg)](https://codeclimate.com/github/charliesome/better_errors) +[![Build Status](https://travis-ci.org/BetterErrors/better_errors.svg)](https://travis-ci.org/BetterErrors/better_errors) +[![Codacy Badge](https://api.codacy.com/project/badge/Grade/6bc3e7d6118d47e6959b16690b815909)](https://www.codacy.com/app/BetterErrors/better_errors?utm_source=github.com&utm_medium=referral&utm_content=BetterErrors/better_errors&utm_campaign=Badge_Grade) +[![Coverage](https://coveralls.io/repos/github/BetterErrors/better_errors/badge.svg?branch=master)](https://coveralls.io/github/BetterErrors/better_errors?branch=master) +[![Gem Version](https://img.shields.io/gem/v/better_errors.svg)](https://rubygems.org/gems/better_errors) + +# Better Errors Better Errors replaces the standard Rails error page with a much better and more useful error page. It is also usable outside of Rails in any Rack app as Rack middleware. -![image](https://i.imgur.com/6zBGAAb.png) +![screenshot of Better Errors in action](https://i.imgur.com/6zBGAAb.png) ## Features +For screenshots of these features, [see the wiki](https://github.com/BetterErrors/better_errors/wiki). + * Full stack trace * Source code inspection for all stack frames (with highlighting) * Local and instance variable inspection -* Live REPL on every stack frame +* Live shell (REPL) on every stack frame +* Links directly to the source line in your editor +* Useful information in non-HTML requests ## Installation @@ -18,76 +27,68 @@ Add this to your Gemfile: ```ruby group :development do gem "better_errors" + gem "binding_of_caller" end ``` -If you would like to use Better Errors' **advanced features** (REPL, local/instance variable inspection, pretty stack frame names), you need to add the [`binding_of_caller`](https://github.com/banister/binding_of_caller) gem by [@banisterfiend](https://twitter.com/banisterfiend) to your Gemfile: +[`binding_of_caller`](https://github.com/banister/binding_of_caller) is optional, but is necessary to use Better Errors' advanced features (REPL, local/instance variable inspection, pretty stack frame names). -```ruby -gem "binding_of_caller" -``` +_Note: If you discover that Better Errors isn't working - particularly after upgrading from version 0.5.0 or less - be sure to set `config.consider_all_requests_local = true` in `config/environments/development.rb`._ -This is an optional dependency however, and Better Errors will work without it. +### Optional: Set `EDITOR` -_Note: If you discover that Better Errors isn't working - particularly after upgrading from version 0.5.0 or less - be sure to set `config.consider_all_requests_local = true` in `config/environments/development.rb`._ +For many reasons outside of Better Errors, you should have the `EDITOR` environment variable set to your preferred +editor. +Better Errors, like many other tools, will use that environment variable to show a link that opens your +editor to the file and line from the console. -## Security +By default the links will open TextMate-protocol links. -**NOTE:** It is *critical* you put better\_errors in the **development** section. **Do NOT run better_errors in production, or on Internet facing hosts.** +To see if your editor is supported or to set up a different editor, see [the wiki](https://github.com/BetterErrors/better_errors/wiki/Link-to-your-editor). -You will notice that the only machine that gets the Better Errors page is localhost, which means you get the default error page if you are developing on a remote host (or a virtually remote host, such as a Vagrant box). Obviously, the REPL is not something you want to expose to the public, but there may also be other pieces of sensitive information available in the backtrace. +### Optional: Set `BETTER_ERRORS_INSIDE_FRAME` -To poke selective holes in this security mechanism, you can add a line like this to your startup (for example, on Rails it would be `config/environments/development.rb`) +If your application is running inside of an iframe, or if you have a Content Security Policy that disallows links +to other protocols, the editor links will not work. -```ruby -BetterErrors::Middleware.allow_ip! ENV['TRUSTED_IP'] if ENV['TRUSTED_IP'] -``` +To work around this set `BETTER_ERRORS_INSIDE_FRAME=1` in the environment, and the links will include `target=_blank`, +allowing the link to open regardless of the policy. -Then run Rails like this: +_This works because it opens the editor from a new browser tab, escaping from the restrictions of your site._ +_Unfortunately it leaves behind an empty tab each time, so only use this if needed._ -```shell -TRUSTED_IP=66.68.96.220 rails s -``` +## Security -Note that the `allow_ip!` is actually backed by a `Set`, so you can add more than one IP address or subnet. +**NOTE:** It is *critical* you put better\_errors only in the **development** section of your Gemfile. +**Do NOT run better_errors in production, or on Internet-facing hosts.** -**Tip:** You can find your apparent IP by hitting the old error page's "Show env dump" and looking at "REMOTE_ADDR". +You will notice that the only machine that gets the Better Errors page is localhost, which means you get the default error page if you are developing on a remote host (or a virtually remote host, such as a Vagrant box). +Obviously, the REPL is not something you want to expose to the public, and there may be sensitive information available in the backtrace. -**VirtualBox:** If you are using VirtualBox and are accessing the guest from your host's browser, you will need to use `allow_ip!` to see the error page. +For more information on how to configure access, see [the wiki](https://github.com/BetterErrors/better_errors/wiki/Allowing-access-to-the-console). ## Usage If you're using Rails, there's nothing else you need to do. -If you're not using Rails, you need to insert `BetterErrors::Middleware` into your middleware stack, and optionally set `BetterErrors.application_root` if you'd like Better Errors to abbreviate filenames within your application. - -Here's an example using Sinatra: +### Using without Rails. -```ruby -require "sinatra" -require "better_errors" - -configure :development do - use BetterErrors::Middleware - BetterErrors.application_root = __dir__ -end +If you're not using Rails, you need to insert `BetterErrors::Middleware` into your middleware stack, and optionally set `BetterErrors.application_root` if you'd like Better Errors to abbreviate filenames within your application. -get "/" do - raise "oops" -end -``` +For instructions for your specific middleware, [see the wiki](https://github.com/BetterErrors/better_errors/wiki/Non-Rails-frameworks). -### Plain text +### Plain text requests Better Errors will render a plain text error page when the request is an `XMLHttpRequest` or when the `Accept` header does *not* include 'html'. ### Unicorn, Puma, and other multi-worker servers -Better Errors works by leaving a lot of context in server process memory. If -you're using a web server that runs multiple "workers" it's likely that a second +Better Errors works by leaving a lot of context in server process memory. +If you're using a web server that runs multiple "workers" it's likely that a second request (as happens when you click on a stack frame) will hit a different -worker. That worker won't have the necessary context in memory, and you'll see +worker. +That worker won't have the necessary context in memory, and you'll see a `Session Expired` message. If this is the case for you, consider turning the number of workers to one (1) @@ -95,10 +96,64 @@ in `development`. Another option would be to use Webrick, Mongrel, Thin, or another single-process server as your `rails server`, when you are trying to troubleshoot an issue in development. +### Changing the link to your editor + +Better Errors includes a link to your editor for the file and line of code that is being shown. +By default, it uses your environment to determine which editor should be opened. +See [the wiki for instructions on configuring the editor](https://github.com/BetterErrors/better_errors/wiki/Link-to-your-editor). + + +## Set maximum variable size for inspector. + +```ruby +# e.g. in config/initializers/better_errors.rb +# This will stop BetterErrors from trying to render larger objects, which can cause +# slow loading times and browser performance problems. Stated size is in characters and refers +# to the length of #inspect's payload for the given object. Please be aware that HTML escaping +# modifies the size of this payload so setting this limit too precisely is not recommended. +# default value: 100_000 +BetterErrors.maximum_variable_inspect_size = 100_000 +``` + +## Ignore inspection of variables with certain classes. + +```ruby +# e.g. in config/initializers/better_errors.rb +# This will stop BetterErrors from trying to inspect objects of these classes, which can cause +# slow loading times and unneccessary database queries. Does not check inheritance chain, use +# strings not contants. +# default value: ['ActionDispatch::Request', 'ActionDispatch::Response'] +BetterErrors.ignored_classes = ['ActionDispatch::Request', 'ActionDispatch::Response'] +``` + ## Get in touch! If you're using better_errors, I'd love to hear from you. Drop me a line and tell me what you think! +## Development + +After checking out the repo, run `bundle install` to install the basic dependencies. + +You can run the tests with the simplest set of dependencies using: + +```rb +bundle exec rspec +``` + +To run specs for each of the dependency combinations, run: + +```rb +bundle exec rake test:all +``` + +You can run specs for a specific dependency combination using: + +```rb +BUNDLE_GEMFILE=gemfiles/pry09.gemfile bundle exec rspec +``` + +On CI, the specs are run against each gemfile on each supported version of Ruby. + ## Contributing 1. Fork it diff --git a/Rakefile b/Rakefile index b6329726..cb84ee87 100644 --- a/Rakefile +++ b/Rakefile @@ -1,13 +1,38 @@ require "bundler/gem_tasks" require "rspec/core/rake_task" -namespace :test do - RSpec::Core::RakeTask.new(:with_binding_of_caller) +RSpec::Core::RakeTask.new(:test) +task :default => :test - without_task = RSpec::Core::RakeTask.new(:without_binding_of_caller) - without_task.ruby_opts = "-I spec -r without_binding_of_caller" +def gemfiles + @gemfiles ||= Dir[File.dirname(__FILE__) + '/gemfiles/*.gemfile'] +end - task :all => [:with_binding_of_caller, :without_binding_of_caller] +def with_each_gemfile + gemfiles.each do |gemfile| + Bundler.with_clean_env do + puts "\n=========== Using gemfile: #{gemfile}" + ENV['BUNDLE_GEMFILE'] = gemfile + yield + end + end end -task :default => "test:all" +namespace :test do + namespace :bundles do + desc "Install all dependencies necessary to test" + task :install do + with_each_gemfile { sh "bundle install" } + end + + desc "Update all dependencies for tests" + task :update do + with_each_gemfile { sh "bundle update" } + end + end + + desc "Test all supported sets of dependencies." + task :all => 'test:bundles:install' do + with_each_gemfile { sh "bundle exec rspec" rescue nil } + end +end diff --git a/better_errors.gemspec b/better_errors.gemspec index 78d2b97d..ef996b0d 100644 --- a/better_errors.gemspec +++ b/better_errors.gemspec @@ -9,20 +9,39 @@ Gem::Specification.new do |s| s.email = ["charlie@charliesomerville.com"] s.description = %q{Provides a better error page for Rails and other Rack apps. Includes source code inspection, a live REPL and local/instance variable inspection for all stack frames.} s.summary = %q{Better error page for Rails and other Rack apps} - s.homepage = "https://github.com/charliesome/better_errors" + s.homepage = "https://github.com/BetterErrors/better_errors" s.license = "MIT" - s.files = `git ls-files`.split($/) - s.test_files = s.files.grep(%r{^(test|spec|features)/}) + s.files = `git ls-files -z`.split("\x0").reject do |f| + f.match(%r{^((test|spec|features|feature-screenshots)/|Rakefile)}) + end + s.require_paths = ["lib"] s.required_ruby_version = ">= 2.0.0" - s.add_dependency "erubis", ">= 2.6.6" + s.add_development_dependency "rake", "~> 10.0" + s.add_development_dependency "rspec", "~> 3.5" + s.add_development_dependency "rspec-html-matchers" + s.add_development_dependency "rspec-its" + s.add_development_dependency "yard" + # kramdown 2.1 requires Ruby 2.3+ + s.add_development_dependency "kramdown", (RUBY_VERSION < '2.3' ? '< 2.0.0' : '> 2.0.0') + # simplecov and coveralls must not be included here. See the Gemfiles instead. + + s.add_dependency "erubi", ">= 1.0.0" s.add_dependency "coderay", ">= 1.0.0" s.add_dependency "rack", ">= 0.9.0" # optional dependencies: # s.add_dependency "binding_of_caller" # s.add_dependency "pry" + + if s.respond_to?(:metadata) + s.metadata['changelog_uri'] = 'https://github.com/BetterErrors/better_errors/releases' + s.metadata['source_code_uri'] = 'https://github.com/BetterErrors/better_errors' + s.metadata['bug_tracker_uri'] = 'https://github.com/BetterErrors/better_errors/issues' + else + puts "Your RubyGems does not support metadata. Update if you'd like to make a release." + end end diff --git a/feature-screenshots/1-application-error.jpg b/feature-screenshots/1-application-error.jpg new file mode 100644 index 00000000..7fb00863 Binary files /dev/null and b/feature-screenshots/1-application-error.jpg differ diff --git a/feature-screenshots/2-other-application-frame.jpg b/feature-screenshots/2-other-application-frame.jpg new file mode 100644 index 00000000..b5db9af3 Binary files /dev/null and b/feature-screenshots/2-other-application-frame.jpg differ diff --git a/feature-screenshots/3-live-shell.jpg b/feature-screenshots/3-live-shell.jpg new file mode 100644 index 00000000..f7a71b5c Binary files /dev/null and b/feature-screenshots/3-live-shell.jpg differ diff --git a/feature-screenshots/4-other-frames.jpg b/feature-screenshots/4-other-frames.jpg new file mode 100644 index 00000000..01ffaeec Binary files /dev/null and b/feature-screenshots/4-other-frames.jpg differ diff --git a/feature-screenshots/5-open-editor.jpg b/feature-screenshots/5-open-editor.jpg new file mode 100644 index 00000000..fb0e9e91 Binary files /dev/null and b/feature-screenshots/5-open-editor.jpg differ diff --git a/feature-screenshots/6-local-variables.jpg b/feature-screenshots/6-local-variables.jpg new file mode 100644 index 00000000..0abe97de Binary files /dev/null and b/feature-screenshots/6-local-variables.jpg differ diff --git a/feature-screenshots/7-non-html-requests.jpg b/feature-screenshots/7-non-html-requests.jpg new file mode 100644 index 00000000..3b0b8e5b Binary files /dev/null and b/feature-screenshots/7-non-html-requests.jpg differ diff --git a/feature-screenshots/8-xhr-shows-text-error.jpg b/feature-screenshots/8-xhr-shows-text-error.jpg new file mode 100644 index 00000000..7bf93a88 Binary files /dev/null and b/feature-screenshots/8-xhr-shows-text-error.jpg differ diff --git a/feature-screenshots/9-xhr-error-in-manual-console.jpg b/feature-screenshots/9-xhr-error-in-manual-console.jpg new file mode 100644 index 00000000..0d1727ee Binary files /dev/null and b/feature-screenshots/9-xhr-error-in-manual-console.jpg differ diff --git a/gemfiles/pry010.gemfile b/gemfiles/pry010.gemfile new file mode 100644 index 00000000..4503b67c --- /dev/null +++ b/gemfiles/pry010.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem 'rack', RUBY_VERSION < '2.2.2' ? '~> 1.6' : '~> 2.0' +gem "binding_of_caller" +gem "pry", "~> 0.10.0" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/pry011.gemfile b/gemfiles/pry011.gemfile new file mode 100644 index 00000000..761c5918 --- /dev/null +++ b/gemfiles/pry011.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem 'rack', RUBY_VERSION < '2.2.2' ? '~> 1.6' : '~> 2.0' +gem "pry", "~> 0.11.0pre" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/pry09.gemfile b/gemfiles/pry09.gemfile new file mode 100644 index 00000000..db7fa6bc --- /dev/null +++ b/gemfiles/pry09.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem 'rack', RUBY_VERSION < '2.2.2' ? '~> 1.6' : '~> 2.0' +gem "pry", "~> 0.9.12" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rack.gemfile b/gemfiles/rack.gemfile new file mode 100644 index 00000000..4f2bab73 --- /dev/null +++ b/gemfiles/rack.gemfile @@ -0,0 +1,7 @@ +source "https://rubygems.org" + +gem 'rack', RUBY_VERSION < '2.2.2' ? '~> 1.6' : '~> 2.0' + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rack_boc.gemfile b/gemfiles/rack_boc.gemfile new file mode 100644 index 00000000..0899c028 --- /dev/null +++ b/gemfiles/rack_boc.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem 'rack', RUBY_VERSION < '2.2.2' ? '~> 1.6' : '~> 2.0' +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails42.gemfile b/gemfiles/rails42.gemfile new file mode 100644 index 00000000..ba2e191f --- /dev/null +++ b/gemfiles/rails42.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 4.2.0" +gem 'nokogiri', RUBY_VERSION < '2.1' ? '~> 1.6.0' : '>= 1.7' +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails42_boc.gemfile b/gemfiles/rails42_boc.gemfile new file mode 100644 index 00000000..2e66ac54 --- /dev/null +++ b/gemfiles/rails42_boc.gemfile @@ -0,0 +1,10 @@ +source "https://rubygems.org" + +gem "rails", "~> 4.2.0" +gem 'nokogiri', RUBY_VERSION < '2.1' ? '~> 1.6.0' : '>= 1.7' +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails42_haml.gemfile b/gemfiles/rails42_haml.gemfile new file mode 100644 index 00000000..a22370a6 --- /dev/null +++ b/gemfiles/rails42_haml.gemfile @@ -0,0 +1,10 @@ +source "https://rubygems.org" + +gem "rails", "~> 4.2.0" +gem 'nokogiri', RUBY_VERSION < '2.1' ? '~> 1.6.0' : '>= 1.7' +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "haml" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails50.gemfile b/gemfiles/rails50.gemfile new file mode 100644 index 00000000..7d31f08d --- /dev/null +++ b/gemfiles/rails50.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.0.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails50_boc.gemfile b/gemfiles/rails50_boc.gemfile new file mode 100644 index 00000000..a7955aad --- /dev/null +++ b/gemfiles/rails50_boc.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.0.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails50_haml.gemfile b/gemfiles/rails50_haml.gemfile new file mode 100644 index 00000000..5ad597ec --- /dev/null +++ b/gemfiles/rails50_haml.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.0.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "haml" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails51.gemfile b/gemfiles/rails51.gemfile new file mode 100644 index 00000000..5532febb --- /dev/null +++ b/gemfiles/rails51.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.1.0" +gem 'i18n', '< 1.5.2', require: false if RUBY_VERSION < '2.3' + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails51_boc.gemfile b/gemfiles/rails51_boc.gemfile new file mode 100644 index 00000000..bfb2cc92 --- /dev/null +++ b/gemfiles/rails51_boc.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.1.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails51_haml.gemfile b/gemfiles/rails51_haml.gemfile new file mode 100644 index 00000000..a870a7cf --- /dev/null +++ b/gemfiles/rails51_haml.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.1.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "haml" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails52.gemfile b/gemfiles/rails52.gemfile new file mode 100644 index 00000000..070970cd --- /dev/null +++ b/gemfiles/rails52.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.2.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails52_boc.gemfile b/gemfiles/rails52_boc.gemfile new file mode 100644 index 00000000..195e4e23 --- /dev/null +++ b/gemfiles/rails52_boc.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.2.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails52_haml.gemfile b/gemfiles/rails52_haml.gemfile new file mode 100644 index 00000000..dd9f0dc9 --- /dev/null +++ b/gemfiles/rails52_haml.gemfile @@ -0,0 +1,9 @@ +source "https://rubygems.org" + +gem "rails", "~> 5.2.0" +gem 'i18n', '< 1.5.2' if RUBY_VERSION < '2.3' +gem "haml" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails60.gemfile b/gemfiles/rails60.gemfile new file mode 100644 index 00000000..55f89c5e --- /dev/null +++ b/gemfiles/rails60.gemfile @@ -0,0 +1,7 @@ +source "https://rubygems.org" + +gem "rails", "~> 6.0.0" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails60_boc.gemfile b/gemfiles/rails60_boc.gemfile new file mode 100644 index 00000000..190c573c --- /dev/null +++ b/gemfiles/rails60_boc.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem "rails", "~> 6.0.0" +gem "binding_of_caller" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/gemfiles/rails60_haml.gemfile b/gemfiles/rails60_haml.gemfile new file mode 100644 index 00000000..84d3401a --- /dev/null +++ b/gemfiles/rails60_haml.gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gem "rails", "~> 6.0.0" +gem "haml" + +gem 'coveralls', require: false + +gemspec path: "../" diff --git a/lib/better_errors.rb b/lib/better_errors.rb index 976927f4..8cd6b7ec 100644 --- a/lib/better_errors.rb +++ b/lib/better_errors.rb @@ -1,15 +1,16 @@ require "pp" -require "erubis" +require "erubi" require "coderay" require "uri" +require "better_errors/version" require "better_errors/code_formatter" +require "better_errors/inspectable_value" require "better_errors/error_page" require "better_errors/middleware" require "better_errors/raised_exception" require "better_errors/repl" require "better_errors/stack_frame" -require "better_errors/version" module BetterErrors POSSIBLE_EDITOR_PRESETS = [ @@ -17,6 +18,11 @@ module BetterErrors { symbols: [:macvim, :mvim], sniff: /vim/i, url: proc { |file, line| "mvim://open?url=file://#{file}&line=#{line}" } }, { symbols: [:sublime, :subl, :st], sniff: /subl/i, url: "subl://open?url=file://%{file}&line=%{line}" }, { symbols: [:textmate, :txmt, :tm], sniff: /mate/i, url: "txmt://open?url=file://%{file}&line=%{line}" }, + { symbols: [:idea], sniff: /idea/i, url: "idea://open?file=%{file}&line=%{line}" }, + { symbols: [:rubymine], sniff: /mine/i, url: "x-mine://open?file=%{file}&line=%{line}" }, + { symbols: [:vscode, :code], sniff: /code/i, url: "vscode://file/%{file}:%{line}" }, + { symbols: [:vscodium, :codium], sniff: /codium/i, url: "vscodium://file/%{file}:%{line}" }, + { symbols: [:atom], sniff: /atom/i, url: "atom://core/open/file?filename=%{file}&line=%{line}" }, ] class << self @@ -44,8 +50,19 @@ class << self # The ignored instance variables. # @return [Array] attr_accessor :ignored_instance_variables + + # The maximum variable payload size. If variable.inspect exceeds this, + # the variable won't be returned. + # @return int + attr_accessor :maximum_variable_inspect_size + + # List of classes that are excluded from inspection. + # @return [Array] + attr_accessor :ignored_classes end @ignored_instance_variables = [] + @maximum_variable_inspect_size = 100_000 + @ignored_classes = ['ActionDispatch::Request', 'ActionDispatch::Response'] # Returns a proc, which when called with a filename and line number argument, # returns a URL to open the filename and line in the selected editor. @@ -68,6 +85,7 @@ def self.editor # * `:textmate`, `:txmt`, `:tm` # * `:sublime`, `:subl`, `:st` # * `:macvim` + # * `:atom` # # @param [Symbol] sym # @@ -117,7 +135,7 @@ def self.editor=(editor) # Enables experimental Pry support in the inline REPL # # If you encounter problems while using Pry, *please* file a bug report at - # https://github.com/charliesome/better_errors/issues + # https://github.com/BetterErrors/better_errors/issues def self.use_pry! REPL::PROVIDERS.unshift const: :Pry, impl: "better_errors/repl/pry" end diff --git a/lib/better_errors/error_page.rb b/lib/better_errors/error_page.rb index a7d888f9..0eb1081d 100644 --- a/lib/better_errors/error_page.rb +++ b/lib/better_errors/error_page.rb @@ -10,7 +10,7 @@ def self.template_path(template_name) end def self.template(template_name) - Erubis::EscapedEruby.new(File.read(template_path(template_name))) + Erubi::Engine.new(File.read(template_path(template_name)), escape: true) end attr_reader :exception, :env, :repls @@ -26,8 +26,13 @@ def id @id ||= SecureRandom.hex(8) end - def render(template_name = "main") - self.class.template(template_name).result binding + def render(template_name = "main", csrf_token = nil) + binding.eval(self.class.template(template_name).src) + rescue => e + # Fix the backtrace, which doesn't identify the template that failed (within Better Errors). + # We don't know the line number, so just injecting the template path has to be enough. + e.backtrace.unshift "#{self.class.template_path(template_name)}:0" + raise end def do_variables(opts) @@ -41,23 +46,43 @@ def do_eval(opts) index = opts["index"].to_i code = opts["source"] - unless binding = backtrace_frames[index].frame_binding + unless (binding = backtrace_frames[index].frame_binding) return { error: "REPL unavailable in this stack frame" } end - result, prompt, prefilled_input = - (@repls[index] ||= REPL.provider.new(binding)).send_input(code) + @repls[index] ||= REPL.provider.new(binding, exception) - { result: result, - prompt: prompt, - prefilled_input: prefilled_input, - highlighted_input: CodeRay.scan(code, :ruby).div(wrap: nil) } + eval_and_respond(index, code) end def backtrace_frames exception.backtrace end + def exception_type + exception.type + end + + def exception_message + exception.message.strip.gsub(/(\r?\n\s*\r?\n)+/, "\n") + end + + def exception_hint + exception.hint + end + + def active_support_actions + return [] unless defined?(ActiveSupport::ActionableError) + + ActiveSupport::ActionableError.actions(exception.type) + end + + def action_dispatch_action_endpoint + return unless defined?(ActionDispatch::ActionableExceptions) + + ActionDispatch::ActionableExceptions.endpoint + end + def application_frames backtrace_frames.select(&:application?) end @@ -66,7 +91,8 @@ def first_frame application_frames.first || backtrace_frames.first end - private + private + def editor_url(frame) BetterErrors.editor[frame.filename, frame.line] end @@ -100,11 +126,30 @@ def text_heading(char, str) end def inspect_value(obj) - CGI.escapeHTML(obj.inspect) - rescue NoMethodError - "(object doesn't support inspect)" - rescue Exception - "(exception was raised in inspect)" + if BetterErrors.ignored_classes.include? obj.class.name + "(Instance of ignored class. "\ + "#{obj.class.name ? "Remove #{CGI.escapeHTML(obj.class.name)} from" : "Modify"}"\ + " BetterErrors.ignored_classes if you need to see it.)" + else + InspectableValue.new(obj).to_html + end + rescue BetterErrors::ValueLargerThanConfiguredMaximum + "(Object too large. "\ + "#{obj.class.name ? "Modify #{CGI.escapeHTML(obj.class.name)}#inspect or a" : "A"}"\ + "djust BetterErrors.maximum_variable_inspect_size if you need to see it.)" + rescue Exception => e + "(exception #{CGI.escapeHTML(e.class.to_s)} was raised in inspect)" + end + + def eval_and_respond(index, code) + result, prompt, prefilled_input = @repls[index].send_input(code) + + { + highlighted_input: CodeRay.scan(code, :ruby).div(wrap: nil), + prefilled_input: prefilled_input, + prompt: prompt, + result: result + } end end end diff --git a/lib/better_errors/inspectable_value.rb b/lib/better_errors/inspectable_value.rb new file mode 100644 index 00000000..b62beb6e --- /dev/null +++ b/lib/better_errors/inspectable_value.rb @@ -0,0 +1,45 @@ +require "cgi" +require "objspace" rescue nil + +module BetterErrors + class ValueLargerThanConfiguredMaximum < StandardError; end + + class InspectableValue + def initialize(value) + @original_value = value + end + + def to_html + raise ValueLargerThanConfiguredMaximum unless value_small_enough_to_inspect? + value_as_html + end + + private + + attr_reader :original_value + + def value_as_html + @value_as_html ||= CGI.escapeHTML(value) + end + + def value + @value ||= begin + if original_value.respond_to? :inspect + original_value.inspect + else + original_value + end + end + end + + def value_small_enough_to_inspect? + return true if BetterErrors.maximum_variable_inspect_size.nil? + + if defined?(ObjectSpace) && defined?(ObjectSpace.memsize_of) && ObjectSpace.memsize_of(value) + ObjectSpace.memsize_of(value) <= BetterErrors.maximum_variable_inspect_size + else + value_as_html.length <= BetterErrors.maximum_variable_inspect_size + end + end + end +end diff --git a/lib/better_errors/middleware.rb b/lib/better_errors/middleware.rb index 3648db87..3a437819 100644 --- a/lib/better_errors/middleware.rb +++ b/lib/better_errors/middleware.rb @@ -1,5 +1,6 @@ require "json" require "ipaddr" +require "securerandom" require "set" require "rack" @@ -33,12 +34,14 @@ class Middleware # Adds an address to the set of IP addresses allowed to access Better # Errors. def self.allow_ip!(addr) - ALLOWED_IPS << IPAddr.new(addr) + ALLOWED_IPS << (addr.is_a?(IPAddr) ? addr : IPAddr.new(addr)) end allow_ip! "127.0.0.0/8" allow_ip! "::1/128" rescue nil # windows ruby doesn't have ipv6 support + CSRF_TOKEN_COOKIE_NAME = "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token" + # A new instance of BetterErrors::Middleware # # @param app The Rack app/middleware to wrap with Better Errors @@ -72,7 +75,7 @@ def allow_ip?(env) def better_errors_call(env) case env["PATH_INFO"] when %r{/__better_errors/(?.+?)/(?\w+)\z} - internal_call env, $~ + internal_call(env, $~[:id], $~[:method]) when %r{/__better_errors/?\z} show_error_page env else @@ -89,53 +92,130 @@ def protected_app_call(env) end def show_error_page(env, exception=nil) + request = Rack::Request.new(env) + csrf_token = request.cookies[CSRF_TOKEN_COOKIE_NAME] || SecureRandom.uuid + type, content = if @error_page if text?(env) [ 'plain', @error_page.render('text') ] else - [ 'html', @error_page.render ] + [ 'html', @error_page.render('main', csrf_token) ] end else [ 'html', no_errors_page ] end status_code = 500 - if defined? ActionDispatch::ExceptionWrapper + if defined?(ActionDispatch::ExceptionWrapper) && exception status_code = ActionDispatch::ExceptionWrapper.new(env, exception).status_code end - [status_code, { "Content-Type" => "text/#{type}; charset=utf-8" }, [content]] + response = Rack::Response.new(content, status_code, { "Content-Type" => "text/#{type}; charset=utf-8" }) + + unless request.cookies[CSRF_TOKEN_COOKIE_NAME] + response.set_cookie(CSRF_TOKEN_COOKIE_NAME, value: csrf_token, path: "/", httponly: true, same_site: :strict) + end + + # In older versions of Rack, the body returned here is actually a Rack::BodyProxy which seems to be a bug. + # (It contains status, headers and body and does not act like an array of strings.) + # Since we already have status code and body here, there's no need to use the ones in the Rack::Response. + (_status_code, headers, _body) = response.finish + [status_code, headers, [content]] end def text?(env) env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" || - !env["HTTP_ACCEPT"].to_s.include?('html') + !env["HTTP_ACCEPT"].to_s.include?('html') end def log_exception return unless BetterErrors.logger - message = "\n#{@error_page.exception.type} - #{@error_page.exception.message}:\n" - @error_page.backtrace_frames.each do |frame| - message << " #{frame}\n" - end + message = "\n#{@error_page.exception_type} - #{@error_page.exception_message}:\n" + message += backtrace_frames.map { |frame| " #{frame}\n" }.join BetterErrors.logger.fatal message end - def internal_call(env, opts) - if opts[:id] != @error_page.id - return [200, { "Content-Type" => "text/plain; charset=utf-8" }, [JSON.dump(error: "Session expired")]] + def backtrace_frames + if defined?(Rails) && defined?(Rails.backtrace_cleaner) + Rails.backtrace_cleaner.clean @error_page.backtrace_frames.map(&:to_s) + else + @error_page.backtrace_frames end + end + + def internal_call(env, id, method) + return not_found_json_response unless %w[variables eval].include?(method) + return no_errors_json_response unless @error_page + return invalid_error_json_response if id != @error_page.id + + request = Rack::Request.new(env) + return invalid_csrf_token_json_response unless request.cookies[CSRF_TOKEN_COOKIE_NAME] + + request.body.rewind + body = JSON.parse(request.body.read) + return invalid_csrf_token_json_response unless request.cookies[CSRF_TOKEN_COOKIE_NAME] == body['csrfToken'] - env["rack.input"].rewind - response = @error_page.send("do_#{opts[:method]}", JSON.parse(env["rack.input"].read)) - [200, { "Content-Type" => "text/plain; charset=utf-8" }, [JSON.dump(response)]] + return not_acceptable_json_response unless request.content_type == 'application/json' + + response = @error_page.send("do_#{method}", body) + [200, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump(response)]] end def no_errors_page "

No errors

No errors have been recorded yet.


" + "Better Errors v#{BetterErrors::VERSION}" end + + def no_errors_json_response + explanation = if defined? Middleman + "Middleman reloads all dependencies for each request, " + + "which breaks Better Errors." + elsif defined?(Shotgun) && defined?(Hanami) + "Hanami is likely running with code-reloading enabled, which is the default. " + + "You can disable this by running hanami with the `--no-code-reloading` option." + elsif defined? Shotgun + "The shotgun gem causes everything to be reloaded for every request. " + + "You can disable shotgun in the Gemfile temporarily to use Better Errors." + else + "The application has been restarted since this page loaded, " + + "or the framework is reloading all gems before each request " + end + [200, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump( + error: 'No exception information available', + explanation: explanation, + )]] + end + + def invalid_error_json_response + [200, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump( + error: "Session expired", + explanation: "This page was likely opened from a previous exception, " + + "and the exception is no longer available in memory.", + )]] + end + + def invalid_csrf_token_json_response + [200, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump( + error: "Invalid CSRF Token", + explanation: "The browser session might have been cleared, " + + "or something went wrong.", + )]] + end + + def not_found_json_response + [404, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump( + error: "Not found", + explanation: "Not a recognized internal call.", + )]] + end + + def not_acceptable_json_response + [406, { "Content-Type" => "application/json; charset=utf-8" }, [JSON.dump( + error: "Request not acceptable", + explanation: "The internal request did not match an acceptable content type.", + )]] + end end end diff --git a/lib/better_errors/raised_exception.rb b/lib/better_errors/raised_exception.rb index 39f11c4f..a581b2b2 100644 --- a/lib/better_errors/raised_exception.rb +++ b/lib/better_errors/raised_exception.rb @@ -4,7 +4,18 @@ class RaisedException attr_reader :exception, :message, :backtrace, :hint def initialize(exception) - if exception.respond_to?(:original_exception) && exception.original_exception + if exception.class.name == "ActionView::Template::Error" && exception.respond_to?(:cause) + # Rails 6+ exceptions of this type wrap the "real" exception, and the real exception + # is actually more useful than the ActionView-provided wrapper. Once Better Errors + # supports showing all exceptions in the cause stack, this should go away. Or perhaps + # this can be changed to provide guidance by showing the second error in the cause stack + # under this condition. + exception = exception.cause if exception.cause + elsif exception.respond_to?(:original_exception) && exception.original_exception + # This supports some specific Rails exceptions, and this is not intended to act the same as + # the Ruby's {Exception#cause}. + # It's possible this should only support ActionView::Template::Error, but by not changing + # this we're preserving longstanding behavior of Better Errors with Rails < 6. exception = exception.original_exception end @@ -35,8 +46,13 @@ def setup_backtrace def setup_backtrace_from_bindings @backtrace = exception.__better_errors_bindings_stack.map { |binding| - file = binding.eval "__FILE__" - line = binding.eval "__LINE__" + if binding.respond_to?(:source_location) # Ruby >= 2.6 + file = binding.source_location[0] + line = binding.source_location[1] + else + file = binding.eval "__FILE__" + line = binding.eval "__LINE__" + end name = binding.frame_description StackFrame.new(file, line, name, binding) } diff --git a/lib/better_errors/repl.rb b/lib/better_errors/repl.rb index 1c1092f0..42136e7c 100644 --- a/lib/better_errors/repl.rb +++ b/lib/better_errors/repl.rb @@ -21,7 +21,9 @@ def self.detect end def self.test_provider(provider) - require provider[:impl] + # We must load this file instead of `require`ing it, since during our tests we want the file + # to be reloaded. In practice, this will only be called once, so `require` is not necessary. + load "#{provider[:impl]}.rb" true rescue LoadError false diff --git a/lib/better_errors/repl/basic.rb b/lib/better_errors/repl/basic.rb index 4b355a97..c2144531 100644 --- a/lib/better_errors/repl/basic.rb +++ b/lib/better_errors/repl/basic.rb @@ -1,7 +1,7 @@ module BetterErrors module REPL class Basic - def initialize(binding) + def initialize(binding, _exception) @binding = binding end diff --git a/lib/better_errors/repl/pry.rb b/lib/better_errors/repl/pry.rb index 5653e42c..4390ec7e 100644 --- a/lib/better_errors/repl/pry.rb +++ b/lib/better_errors/repl/pry.rb @@ -30,9 +30,13 @@ def read_buffer ensure @buffer = "" end + + def print(*args) + @buffer << args.join(' ') + end end - def initialize(binding) + def initialize(binding, exception) @fiber = Fiber.new do @pry.repl binding end @@ -40,9 +44,15 @@ def initialize(binding) @output = BetterErrors::REPL::Pry::Output.new @pry = ::Pry.new input: @input, output: @output @pry.hooks.clear_all if defined?(@pry.hooks.clear_all) + store_last_exception exception @fiber.resume end + def store_last_exception(exception) + return unless defined? ::Pry::LastException + @pry.instance_variable_set(:@last_exception, ::Pry::LastException.new(exception.exception)) + end + def send_input(str) local ::Pry.config, color: false, pager: false do @fiber.resume "#{str}\n" diff --git a/lib/better_errors/stack_frame.rb b/lib/better_errors/stack_frame.rb index 2a347f4f..706edaf1 100644 --- a/lib/better_errors/stack_frame.rb +++ b/lib/better_errors/stack_frame.rb @@ -68,15 +68,27 @@ def pretty_path def local_variables return {} unless frame_binding - frame_binding.eval("local_variables").each_with_object({}) do |name, hash| - if defined?(frame_binding.local_variable_get) - hash[name] = frame_binding.local_variable_get(name) - else - hash[name] = frame_binding.eval(name.to_s) - end + + lv = frame_binding.eval("local_variables") + return {} unless lv + + lv.each_with_object({}) do |name, hash| + # Ruby 2.2's local_variables will include the hidden #$! variable if + # called from within a rescue context. This is not a valid variable name, + # so the local_variable_get method complains. This should probably be + # considered a bug in Ruby itself, but we need to work around it. + next if name == :"\#$!" + + hash[name] = local_variable(name) end end + def local_variable(name) + get_local_variable(name) || eval_local_variable(name) + rescue NameError => ex + "#{ex.class}: #{ex.message}" + end + def instance_variables return {} unless frame_binding Hash[visible_instance_variables.map { |x| @@ -85,7 +97,10 @@ def instance_variables end def visible_instance_variables - frame_binding.eval("instance_variables") - BetterErrors.ignored_instance_variables + iv = frame_binding.eval("instance_variables") + return {} unless iv + + iv - BetterErrors.ignored_instance_variables end def to_s @@ -107,5 +122,15 @@ def set_pretty_method_name @method_name = "##{method_name}" end end + + def get_local_variable(name) + if defined?(frame_binding.local_variable_get) + frame_binding.local_variable_get(name) + end + end + + def eval_local_variable(name) + frame_binding.eval(name.to_s) + end end end diff --git a/lib/better_errors/templates/main.erb b/lib/better_errors/templates/main.erb index 35f1fbde..c7c8c6a8 100644 --- a/lib/better_errors/templates/main.erb +++ b/lib/better_errors/templates/main.erb @@ -1,7 +1,7 @@ - <%= exception.type %> at <%= request_path %> + <%= exception_type %> at <%= request_path %> <%# Stylesheets are placed in the for Turbolinks compatibility. %> @@ -146,6 +146,14 @@ } /* Heading */ + header.exception .fix-actions { + margin-top: .5em; + } + + header.exception .fix-actions input[type=submit] { + font-weight: bold; + } + header.exception h2 { font-weight: 200; font-size: 11pt; @@ -153,7 +161,7 @@ header.exception h2, header.exception p { - line-height: 1.4em; + line-height: 1.5em; overflow: hidden; white-space: pre; text-overflow: ellipsis; @@ -166,7 +174,7 @@ header.exception p { font-weight: 200; - font-size: 20pt; + font-size: 17pt; color: white; } @@ -394,7 +402,7 @@ * Monospace * --------------------------------------------------------------------- */ - pre, code, .repl input, .repl .prompt span, textarea, .code_linenums { + pre, code, .be-repl input, .be-repl .command-line span, textarea, .code_linenums { font-family: menlo, lucida console, monospace; font-size: 8pt; } @@ -460,7 +468,7 @@ font-weight: 200; } - .code, .console, .unavailable { + .code, .be-console, .unavailable { background: #fff; padding: 5px; @@ -532,13 +540,13 @@ } /* REPL shell */ - .console { + .be-console { padding: 0 1px 10px 1px; border-bottom-left-radius: 2px; border-bottom-right-radius: 2px; } - .console pre { + .be-console pre { padding: 10px 10px 0 10px; max-height: 400px; overflow-x: none; @@ -548,30 +556,31 @@ white-space: pre-wrap; } - /* .prompt > span + input */ - .console .prompt { + /* .command-line > span + input */ + .be-console .command-line { display: table; width: 100%; } - .console .prompt span, - .console .prompt input { + .be-console .command-line span, + .be-console .command-line input { display: table-cell; } - .console .prompt span { + .be-console .command-line span { width: 1%; padding-right: 5px; padding-left: 10px; + white-space: pre; } - .console .prompt input { + .be-console .command-line input { width: 99%; } /* Input box */ - .console input, - .console input:focus { + .be-console input, + .be-console input:focus { outline: 0; border: 0; padding: 0; @@ -669,14 +678,14 @@ nav.sidebar::-webkit-scrollbar, .inset pre::-webkit-scrollbar, - .console pre::-webkit-scrollbar, + .be-console pre::-webkit-scrollbar, .code::-webkit-scrollbar { width: 10px; height: 10px; } .inset pre::-webkit-scrollbar-thumb, - .console pre::-webkit-scrollbar-thumb, + .be-console pre::-webkit-scrollbar-thumb, .code::-webkit-scrollbar-thumb { background: #ccc; border-radius: 5px; @@ -692,7 +701,7 @@ background: -webkit-linear-gradient(left, #aaa, #999); } - .console pre:hover::-webkit-scrollbar-thumb, + .be-console pre:hover::-webkit-scrollbar-thumb, .inset pre:hover::-webkit-scrollbar-thumb, .code:hover::-webkit-scrollbar-thumb { background: #888; @@ -720,21 +729,43 @@ if(document.styleSheets[i].href) document.styleSheets[i].disabled = true; } - document.addEventListener("page:restore", function restoreCSS(e) { - for(var i=0; i < document.styleSheets.length; i++) { - document.styleSheets[i].disabled = false; - } - document.removeEventListener("page:restore", restoreCSS, false); - }); + if (window.Turbolinks.controller) { + // Turbolinks > 5 (see https://github.com/turbolinks/turbolinks/issues/6) + document.addEventListener("turbolinks:load", function restoreCSS(e) { + for(var i=0; i < document.styleSheets.length; i++) { + document.styleSheets[i].disabled = false; + } + document.removeEventListener("turbolinks:load", restoreCSS, false); + }); + } else { + document.addEventListener("page:restore", function restoreCSS(e) { + for(var i=0; i < document.styleSheets.length; i++) { + document.styleSheets[i].disabled = false; + } + document.removeEventListener("page:restore", restoreCSS, false); + }); + } }
-

<%= exception.type %> at <%= request_path %>

-

<%= exception.message %>

- <% if exception.hint %> -

Hint: <%= exception.hint %>

+

<%= exception_type %> at <%= request_path %>

+

<%= exception_message %>

+ <% unless active_support_actions.empty? %> +
+ <% active_support_actions.each do |action, _| %> +
+ + + + +
+ <% end %> +
+ <% end %> + <% if exception_hint %> +

Hint: <%= exception_hint %>

<% end %>
@@ -772,6 +803,7 @@ (function() { var OID = "<%= id %>"; + var csrfToken = "<%= csrf_token %>"; var previousFrame = null; var previousFrameInfo = null; @@ -780,8 +812,9 @@ function apiCall(method, opts, cb) { var req = new XMLHttpRequest(); - req.open("POST", <%== uri_prefix.gsub("<", "<").inspect %> + "/__better_errors/" + OID + "/" + method, true); + req.open("POST", "//" + window.location.host + <%== uri_prefix.gsub("<", "<").inspect %> + "/__better_errors/" + OID + "/" + method, true); req.setRequestHeader("Content-Type", "application/json"); + opts.csrfToken = csrfToken; req.send(JSON.stringify(opts)); req.onreadystatechange = function() { if(req.readyState == 4) { @@ -812,7 +845,7 @@ REPL.prototype.install = function(containerElement) { this.container = containerElement; - this.promptElement = this.container.querySelector(".prompt span"); + this.promptElement = this.container.querySelector(".command-line .prompt"); this.inputElement = this.container.querySelector("input"); this.outputElement = this.container.querySelector("pre"); @@ -927,7 +960,7 @@ el.style.display = "block"; - var replInput = el.querySelector('.console input'); + var replInput = el.querySelector('.be-console input'); if (replInput) replInput.focus(); } @@ -941,17 +974,20 @@ apiCall("variables", { "index": index }, function(response) { el.loaded = true; if(response.error) { - el.innerHTML = "" + escapeHTML(response.error) + ""; + el.innerHTML = "

" + escapeHTML(response.error) + "

"; + if(response.explanation) { + el.innerHTML += "

" + escapeHTML(response.explanation) + "

"; + } + el.innerHTML += "

More about Better Errors

"; } else { el.innerHTML = response.html; - var repl = el.querySelector(".repl .console"); + var repl = el.querySelector(".be-repl .be-console"); if(repl) { new REPL(index).install(repl); } - - switchTo(el); } + switchTo(el); }); } } diff --git a/lib/better_errors/templates/text.erb b/lib/better_errors/templates/text.erb index fe9068bb..0a99cc4b 100644 --- a/lib/better_errors/templates/text.erb +++ b/lib/better_errors/templates/text.erb @@ -1,7 +1,10 @@ -<%== text_heading("=", "%s at %s" % [exception.type, request_path]) %> +<%== text_heading("=", "%s at %s" % [exception_type, request_path]) %> -> <%== exception.message %> -<% if backtrace_frames.any? %> +<%== exception_message %> + +> To access an interactive console with this error, point your browser to: /__better_errors + +<% if backtrace_frames.any? -%> <%== text_heading("-", "%s, line %i" % [first_frame.pretty_path, first_frame.line]) %> diff --git a/lib/better_errors/templates/variable_info.erb b/lib/better_errors/templates/variable_info.erb index bde5daab..24f21cd5 100644 --- a/lib/better_errors/templates/variable_info.erb +++ b/lib/better_errors/templates/variable_info.erb @@ -1,17 +1,24 @@
<%== html_formatted_code_block @frame %>
<% if BetterErrors.binding_of_caller_available? && @frame.frame_binding %> -
-
+
+

-                
>>
+
>>
<% end %> @@ -45,26 +52,28 @@
-
-

Local Variables

-
- - <% @frame.local_variables.each do |name, value| %> - - <% end %> -
<%= name %>
<%== inspect_value value %>
+<% if BetterErrors.binding_of_caller_available? && @frame.frame_binding %> +
+

Local Variables

+
+ + <% @frame.local_variables.each do |name, value| %> + + <% end %> +
<%= name %>
<%== inspect_value value %>
+
-
-
-

Instance Variables

-
- - <% @frame.instance_variables.each do |name, value| %> - - <% end %> -
<%= name %>
<%== inspect_value value %>
+
+

Instance Variables

+
+ + <% @frame.instance_variables.each do |name, value| %> + + <% end %> +
<%= name %>
<%== inspect_value value %>
+
-
- + +<% end %> diff --git a/lib/better_errors/version.rb b/lib/better_errors/version.rb index 2096403e..1476281a 100644 --- a/lib/better_errors/version.rb +++ b/lib/better_errors/version.rb @@ -1,3 +1,3 @@ module BetterErrors - VERSION = "2.1.1" + VERSION = "2.8.3" end diff --git a/spec/better_errors/code_formatter_spec.rb b/spec/better_errors/code_formatter_spec.rb index 93445b46..5b612de6 100644 --- a/spec/better_errors/code_formatter_spec.rb +++ b/spec/better_errors/code_formatter_spec.rb @@ -7,13 +7,13 @@ module BetterErrors let(:formatter) { CodeFormatter.new(filename, 8) } it "picks an appropriate scanner" do - formatter.coderay_scanner.should == :ruby + expect(formatter.coderay_scanner).to eq(:ruby) end it "shows 5 lines of context" do - formatter.line_range.should == (3..13) + expect(formatter.line_range).to eq(3..13) - formatter.context_lines.should == [ + expect(formatter.context_lines).to eq([ "three\n", "four\n", "five\n", @@ -25,40 +25,40 @@ module BetterErrors "eleven\n", "twelve\n", "thirteen\n" - ] + ]) end it "works when the line is right on the edge" do formatter = CodeFormatter.new(filename, 20) - formatter.line_range.should == (15..20) + expect(formatter.line_range).to eq(15..20) end describe CodeFormatter::HTML do it "highlights the erroring line" do formatter = CodeFormatter::HTML.new(filename, 8) - formatter.output.should =~ /highlight.*eight/ + expect(formatter.output).to match(/highlight.*eight/) end it "works when the line is right on the edge" do formatter = CodeFormatter::HTML.new(filename, 20) - formatter.output.should_not == formatter.source_unavailable + expect(formatter.output).not_to eq(formatter.source_unavailable) end it "doesn't barf when the lines don't make any sense" do formatter = CodeFormatter::HTML.new(filename, 999) - formatter.output.should == formatter.source_unavailable + expect(formatter.output).to eq(formatter.source_unavailable) end it "doesn't barf when the file doesn't exist" do formatter = CodeFormatter::HTML.new("fkdguhskd7e l", 1) - formatter.output.should == formatter.source_unavailable + expect(formatter.output).to eq(formatter.source_unavailable) end end describe CodeFormatter::Text do it "highlights the erroring line" do formatter = CodeFormatter::Text.new(filename, 8) - formatter.output.should == <<-TEXT.gsub(/^ /, "") + expect(formatter.output).to eq <<-TEXT.gsub(/^ /, "") 3 three 4 four 5 five @@ -75,17 +75,17 @@ module BetterErrors it "works when the line is right on the edge" do formatter = CodeFormatter::Text.new(filename, 20) - formatter.output.should_not == formatter.source_unavailable + expect(formatter.output).not_to eq(formatter.source_unavailable) end it "doesn't barf when the lines don't make any sense" do formatter = CodeFormatter::Text.new(filename, 999) - formatter.output.should == formatter.source_unavailable + expect(formatter.output).to eq(formatter.source_unavailable) end it "doesn't barf when the file doesn't exist" do formatter = CodeFormatter::Text.new("fkdguhskd7e l", 1) - formatter.output.should == formatter.source_unavailable + expect(formatter.output).to eq(formatter.source_unavailable) end end end diff --git a/spec/better_errors/error_page_spec.rb b/spec/better_errors/error_page_spec.rb index 0fc99ccb..74d4f2bb 100644 --- a/spec/better_errors/error_page_spec.rb +++ b/spec/better_errors/error_page_spec.rb @@ -1,14 +1,21 @@ require "spec_helper" +class ErrorPageTestIgnoredClass; end + module BetterErrors describe ErrorPage do + # It's necessary to use HTML matchers here that are specific as possible. + # This is because if there's an exception within this file, the lines of code will be reflected in the + # generated HTML, so any strings being matched against the HTML content will be there if they're within 5 + # lines of code of the exception that was raised. + let!(:exception) { raise ZeroDivisionError, "you divided by zero you silly goose!" rescue $! } let(:error_page) { ErrorPage.new exception, { "PATH_INFO" => "/some/path" } } let(:response) { error_page.render } - let(:empty_binding) { + let(:exception_binding) { local_a = :value_for_local_a local_b = :value_for_local_b @@ -19,58 +26,459 @@ module BetterErrors } it "includes the error message" do - response.should include("you divided by zero you silly goose!") + expect(response).to have_tag('.exception p', text: /you divided by zero you silly goose!/) end it "includes the request path" do - response.should include("/some/path") + expect(response).to have_tag('.exception h2', %r{/some/path}) end it "includes the exception class" do - response.should include("ZeroDivisionError") + expect(response).to have_tag('.exception h2', /ZeroDivisionError/) + end + + context 'when ActiveSupport::ActionableError is available' do + before do + skip "ActiveSupport missing on this platform" unless Object.constants.include?(:ActiveSupport) + skip "ActionableError missing on this platform" unless ActiveSupport.constants.include?(:ActionableError) + end + + context 'when ActiveSupport provides one or more actions for this error type' do + let(:exception_class) { + Class.new(StandardError) do + include ActiveSupport::ActionableError + + action "Do a thing" do + puts "Did a thing" + end + end + } + let(:exception) { exception_binding.eval("raise exception_class") rescue $! } + + it "includes a fix-action form for each action" do + expect(response).to have_tag('.fix-actions') do + with_tag('form.button_to') + with_tag('form.button_to input[type=submit][value="Do a thing"]') + end + end + end + + context 'when ActiveSupport does not provide any actions for this error type' do + let(:exception_class) { + Class.new(StandardError) + } + let(:exception) { exception_binding.eval("raise exception_class") rescue $! } + + it "does not include a fix-action form" do + expect(response).not_to have_tag('.fix-actions') + end + end end context "variable inspection" do - let(:exception) { empty_binding.eval("raise") rescue $! } + let(:html) { error_page.do_variables("index" => 0)[:html] } + let(:exception) { exception_binding.eval("raise") rescue $! } - if BetterErrors.binding_of_caller_available? - it "shows local variables" do - html = error_page.do_variables("index" => 0)[:html] - html.should include("local_a") - html.should include(":value_for_local_a") - html.should include("local_b") - html.should include(":value_for_local_b") + it 'includes an editor link for the full path of the current frame' do + expect(html).to have_tag('.location .filename') do + with_tag('a[href*="better_errors"]') end - else - it "tells the user to add binding_of_caller to their gemfile to get fancy features" do - html = error_page.do_variables("index" => 0)[:html] - html.should include(%{gem "binding_of_caller"}) + end + + context 'when BETTER_ERRORS_INSIDE_FRAME is set in the environment' do + before do + ENV['BETTER_ERRORS_INSIDE_FRAME'] = '1' + end + after do + ENV['BETTER_ERRORS_INSIDE_FRAME'] = nil + end + + it 'includes an editor link with target=_blank' do + expect(html).to have_tag('.location .filename') do + with_tag('a[href*="better_errors"][target="_blank"]') + end end end - it "shows instance variables" do - html = error_page.do_variables("index" => 0)[:html] - html.should include("inst_c") - html.should include(":value_for_inst_c") - html.should include("inst_d") - html.should include(":value_for_inst_d") + context 'when BETTER_ERRORS_INSIDE_FRAME is not set in the environment' do + it 'includes an editor link without target=_blank' do + expect(html).to have_tag('.location .filename') do + with_tag('a[href*="better_errors"]:not([target="_blank"])') + end + end end - it "shows filter instance variables" do - BetterErrors.stub(:ignored_instance_variables).and_return([ :@inst_d ]) - html = error_page.do_variables("index" => 0)[:html] - html.should include("inst_c") - html.should include(":value_for_inst_c") - html.should_not include('@inst_d') - html.should_not include("
:value_for_inst_d
") + context "when binding_of_caller is loaded" do + before do + skip "binding_of_caller is not loaded" unless BetterErrors.binding_of_caller_available? + end + + it "shows local variables" do + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: 'local_a') + with_tag('pre', text: ':value_for_local_a') + end + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: 'local_b') + with_tag('pre', text: ':value_for_local_b') + end + end + + it "shows instance variables" do + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@inst_c') + with_tag('pre', text: ':value_for_inst_c') + end + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@inst_d') + with_tag('pre', text: ':value_for_inst_d') + end + end + + context 'when ignored_classes includes the class name of a local variable' do + before do + allow(BetterErrors).to receive(:ignored_classes).and_return(['ErrorPageTestIgnoredClass']) + end + + let(:exception_binding) { + local_a = :value_for_local_a + local_b = ErrorPageTestIgnoredClass.new + + @inst_c = :value_for_inst_c + @inst_d = ErrorPageTestIgnoredClass.new + + binding + } + + it "does not include that value" do + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: 'local_a') + with_tag('pre', text: ':value_for_local_a') + end + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: 'local_b') + with_tag('.unsupported', text: /Instance of ignored class/) + with_tag('.unsupported', text: /BetterErrors\.ignored_classes/) + end + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@inst_c') + with_tag('pre', text: ':value_for_inst_c') + end + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@inst_d') + with_tag('.unsupported', text: /Instance of ignored class/) + with_tag('.unsupported', text: /BetterErrors\.ignored_classes/) + end + end + end + + it "does not show filtered variables" do + allow(BetterErrors).to receive(:ignored_instance_variables).and_return([:@inst_d]) + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@inst_c') + with_tag('pre', text: ':value_for_inst_c') + end + expect(html).not_to have_tag('div.variables td.name', text: '@inst_d') + end + + context 'when maximum_variable_inspect_size is set' do + before do + BetterErrors.maximum_variable_inspect_size = 1010 + end + + context 'on a platform with ObjectSpace' do + before do + skip "Missing on this platform" unless Object.constants.include?(:ObjectSpace) + end + + context 'with a variable that is smaller than maximum_variable_inspect_size' do + let(:exception_binding) { + @small = content + + binding + } + let(:content) { 'A' * 480 } + + it "shows the variable content" do + expect(html).to have_tag('div.variables', text: %r{#{content}}) + end + end + + context 'with a variable that is larger than maximum_variable_inspect_size' do + context 'but has an #inspect that returns a smaller value' do + let(:exception_binding) { + @big = content + + binding + } + let(:content) { + class ExtremelyLargeInspectableTestValue + def initialize + @a = 'A' * 1101 + end + def inspect + "shortval" + end + end + ExtremelyLargeInspectableTestValue.new + } + + it "shows the variable content" do + expect(html).to have_tag('div.variables', text: /shortval/) + end + end + context 'and does not implement #inspect' do + let(:exception_binding) { + @big = content + + binding + } + let(:content) { 'A' * 1101 } + + it "includes an indication that the variable was too large" do + expect(html).not_to have_tag('div.variables', text: %r{#{content}}) + expect(html).to have_tag('div.variables', text: /Object too large/) + end + end + + context "when the variable's class is anonymous" do + let(:exception_binding) { + @big_anonymous = Class.new do + def initialize + @content = 'A' * 1101 + end + end.new + + binding + } + + it "does not attempt to show the class name" do + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@big_anonymous') + with_tag('.unsupported', text: /Object too large/) + with_tag('.unsupported', text: /Adjust BetterErrors.maximum_variable_inspect_size/) + end + end + end + end + end + + context 'on a platform without ObjectSpace' do + before do + Object.send(:remove_const, :ObjectSpace) if Object.constants.include?(:ObjectSpace) + end + after do + require "objspace" rescue nil + end + + context 'with a variable that is smaller than maximum_variable_inspect_size' do + let(:exception_binding) { + @small = content + + binding + } + let(:content) { 'A' * 480 } + + it "shows the variable content" do + expect(html).to have_tag('div.variables', text: %r{#{content}}) + end + end + + context 'with a variable that is larger than maximum_variable_inspect_size' do + context 'but has an #inspect that returns a smaller value' do + let(:exception_binding) { + @big = content + + binding + } + let(:content) { + class ExtremelyLargeInspectableTestValue + def initialize + @a = 'A' * 1101 + end + def inspect + "shortval" + end + end + ExtremelyLargeInspectableTestValue.new + } + + it "shows the variable content" do + expect(html).to have_tag('div.variables', text: /shortval/) + end + end + context 'and does not implement #inspect' do + let(:exception_binding) { + @big = content + + binding + } + let(:content) { 'A' * 1101 } + + it "includes an indication that the variable was too large" do + + expect(html).not_to have_tag('div.variables', text: %r{#{content}}) + expect(html).to have_tag('div.variables', text: /Object too large/) + end + end + end + + context "when the variable's class is anonymous" do + let(:exception_binding) { + @big_anonymous = Class.new do + def initialize + @content = 'A' * 1101 + end + end.new + + binding + } + + it "does not attempt to show the class name" do + expect(html).to have_tag('div.variables tr') do + with_tag('td.name', text: '@big_anonymous') + with_tag('.unsupported', text: /Object too large/) + with_tag('.unsupported', text: /Adjust BetterErrors.maximum_variable_inspect_size/) + end + end + end + end + end + + context 'when maximum_variable_inspect_size is disabled' do + before do + BetterErrors.maximum_variable_inspect_size = nil + end + + let(:exception_binding) { + @big = content + + binding + } + let(:content) { 'A' * 100_001 } + + it "includes the content of large variables" do + expect(html).to have_tag('div.variables', text: %r{#{content}}) + expect(html).not_to have_tag('div.variables', text: /Object too large/) + end + end + end + + context "when binding_of_caller is not loaded" do + before do + skip "binding_of_caller is loaded" if BetterErrors.binding_of_caller_available? + end + + it "tells the user to add binding_of_caller to their gemfile to get fancy features" do + expect(html).not_to have_tag('div.variables', text: /gem "binding_of_caller"/) + end end end it "doesn't die if the source file is not a real filename" do - exception.stub(:backtrace).and_return([ + allow(exception).to receive(:__better_errors_bindings_stack).and_return([]) + allow(exception).to receive(:backtrace).and_return([ ":10:in `spawn_rack_application'" ]) - response.should include("Source unavailable") + expect(response).to have_tag('.frames li .location .filename', text: '') + end + + context 'with an exception with blank lines' do + class SpacedError < StandardError + def initialize(message = nil) + message = "\n\n#{message}" if message + super + end + end + + let!(:exception) { raise SpacedError, "Danger Warning!" rescue $! } + + it 'does not include leading blank lines in exception_message' do + expect(exception.message).to match(/\A\n\n/) + expect(error_page.exception_message).not_to match(/\A\n\n/) + end + end + + describe '#do_eval' do + let(:exception) { exception_binding.eval("raise") rescue $! } + subject(:do_eval) { error_page.do_eval("index" => 0, "source" => command) } + let(:command) { 'EvalTester.stuff_was_done(:yep)' } + before do + stub_const('EvalTester', eval_tester) + end + let(:eval_tester) { double('EvalTester', stuff_was_done: 'response') } + + context 'without binding_of_caller' do + before do + skip("Disabled with binding_of_caller") if defined? ::BindingOfCaller + end + + it "does not evaluate the code" do + do_eval + expect(eval_tester).to_not have_received(:stuff_was_done).with(:yep) + end + + it 'returns an error indicating no REPL' do + expect(do_eval).to include( + error: "REPL unavailable in this stack frame", + ) + end + end + + context 'with binding_of_caller available' do + before do + skip("Disabled without binding_of_caller") unless defined? ::BindingOfCaller + end + + context 'with Pry disabled or unavailable' do + it "evaluates the code" do + do_eval + expect(eval_tester).to have_received(:stuff_was_done).with(:yep) + end + + it 'returns a hash of the code and its result' do + expect(do_eval).to include( + highlighted_input: /stuff_was_done/, + prefilled_input: '', + prompt: '>>', + result: "=> \"response\"\n", + ) + end + end + + context 'with Pry enabled' do + before do + skip("Disabled without pry") unless defined? ::Pry + + BetterErrors.use_pry! + # Cause the provider to be unselected, so that it will be re-detected. + BetterErrors::REPL.provider = nil + end + after do + BetterErrors::REPL::PROVIDERS.shift + BetterErrors::REPL.provider = nil + + # Ensure the Pry REPL file has not been included. If this is not done, + # the constant leaks into other examples. + BetterErrors::REPL.send(:remove_const, :Pry) + end + + it "evaluates the code" do + BetterErrors::REPL.provider + do_eval + expect(eval_tester).to have_received(:stuff_was_done).with(:yep) + end + + it 'returns a hash of the code and its result' do + expect(do_eval).to include( + highlighted_input: /stuff_was_done/, + prefilled_input: '', + prompt: '>>', + result: "=> \"response\"\n", + ) + end + end + end end end end diff --git a/spec/better_errors/middleware_spec.rb b/spec/better_errors/middleware_spec.rb index 4cdd7826..0cdd8216 100644 --- a/spec/better_errors/middleware_spec.rb +++ b/spec/better_errors/middleware_spec.rb @@ -4,44 +4,50 @@ module BetterErrors describe Middleware do let(:app) { Middleware.new(->env { ":)" }) } let(:exception) { RuntimeError.new("oh no :(") } + let(:status) { response_env[0] } + let(:headers) { response_env[1] } + let(:body) { response_env[2].join } - it "passes non-error responses through" do - app.call({}).should == ":)" + context 'when the application raises no exception' do + it "passes non-error responses through" do + expect(app.call({})).to eq(":)") + end end it "calls the internal methods" do - app.should_receive :internal_call + expect(app).to receive :internal_call app.call("PATH_INFO" => "/__better_errors/1/preform_awesomness") end it "calls the internal methods on any subfolder path" do - app.should_receive :internal_call + expect(app).to receive :internal_call app.call("PATH_INFO" => "/any_sub/folder/path/__better_errors/1/preform_awesomness") end it "shows the error page" do - app.should_receive :show_error_page + expect(app).to receive :show_error_page app.call("PATH_INFO" => "/__better_errors/") end - it "shows the error page on any subfolder path" do - app.should_receive :show_error_page - app.call("PATH_INFO" => "/any_sub/folder/path/__better_errors/") - end - it "doesn't show the error page to a non-local address" do - app.should_not_receive :better_errors_call + expect(app).not_to receive :better_errors_call app.call("REMOTE_ADDR" => "1.2.3.4") end it "shows to a whitelisted IP" do BetterErrors::Middleware.allow_ip! '77.55.33.11' - app.should_receive :better_errors_call + expect(app).to receive :better_errors_call + app.call("REMOTE_ADDR" => "77.55.33.11") + end + + it "shows to a whitelisted IPAddr" do + BetterErrors::Middleware.allow_ip! IPAddr.new('77.55.33.0/24') + expect(app).to receive :better_errors_call app.call("REMOTE_ADDR" => "77.55.33.11") end it "respects the X-Forwarded-For header" do - app.should_not_receive :better_errors_call + expect(app).not_to receive :better_errors_call app.call( "REMOTE_ADDR" => "127.0.0.1", "HTTP_X_FORWARDED_FOR" => "1.2.3.4", @@ -56,30 +62,97 @@ module BetterErrors expect { app.call("REMOTE_ADDR" => "0:0:0:0:0:0:0:1%0" ) }.to_not raise_error end - context "when requesting the /__better_errors manually" do - let(:app) { Middleware.new(->env { ":)" }) } + context "when /__better_errors is requested directly" do + let(:response_env) { app.call("PATH_INFO" => "/__better_errors") } + + context "when no error has been recorded since startup" do + it "shows that no errors have been recorded" do + expect(body).to match /No errors have been recorded yet./ + end - it "shows that no errors have been recorded" do - status, headers, body = app.call("PATH_INFO" => "/__better_errors") - body.join.should match /No errors have been recorded yet./ + it 'does not attempt to use ActionDispatch::ExceptionWrapper on the nil exception' do + ad_ew = double("ActionDispatch::ExceptionWrapper") + stub_const('ActionDispatch::ExceptionWrapper', ad_ew) + expect(ad_ew).to_not receive :new + + response_env + end + + context 'when requested inside a subfolder path' do + let(:response_env) { app.call("PATH_INFO" => "/any_sub/folder/__better_errors") } + + it "shows that no errors have been recorded" do + expect(body).to match /No errors have been recorded yet./ + end + end end - it "shows that no errors have been recorded on any subfolder path" do - status, headers, body = app.call("PATH_INFO" => "/any_sub/folder/path/__better_errors") - body.join.should match /No errors have been recorded yet./ + context 'when an error has been recorded' do + let(:app) { + Middleware.new(->env do + # Only raise on the first request + raise exception unless @already_raised + @already_raised = true + end) + } + before do + app.call({}) + end + + it 'returns the information of the most recent error' do + expect(body).to include("oh no :(") + end + + it 'does not attempt to use ActionDispatch::ExceptionWrapper' do + ad_ew = double("ActionDispatch::ExceptionWrapper") + stub_const('ActionDispatch::ExceptionWrapper', ad_ew) + expect(ad_ew).to_not receive :new + + response_env + end + + context 'when inside a subfolder path' do + let(:response_env) { app.call("PATH_INFO" => "/any_sub/folder/__better_errors") } + + it "shows the error page on any subfolder path" do + expect(app).to receive :show_error_page + app.call("PATH_INFO" => "/any_sub/folder/path/__better_errors/") + end + end end end context "when handling an error" do let(:app) { Middleware.new(->env { raise exception }) } + let(:response_env) { app.call({}) } it "returns status 500" do - status, headers, body = app.call({}) + expect(status).to eq(500) + end + + context "when the exception has a cause" do + before do + pending "This Ruby does not support `cause`" unless Exception.new.respond_to?(:cause) + end + + let(:app) { + Middleware.new(->env { + begin + raise "First Exception" + rescue + raise "Second Exception" + end + }) + } - status.should == 500 + it "shows the exception as-is" do + expect(status).to eq(500) + expect(body).to match(/\nSecond Exception\n/) + expect(body).not_to match(/\nFirst Exception\n/) + end end - context "original_exception" do + context "when the exception responds to #original_exception" do class OriginalExceptionException < Exception attr_reader :original_exception @@ -89,65 +162,394 @@ def initialize(message, original_exception = nil) end end - it "shows Original Exception if it responds_to and has an original_exception" do - app = Middleware.new(->env { - raise OriginalExceptionException.new("Other Exception", Exception.new("Original Exception")) - }) - - status, _, body = app.call({}) - - status.should == 500 - body.join.should_not match(/Other Exception/) - body.join.should match(/Original Exception/) + context 'and has one' do + let(:app) { + Middleware.new(->env { + raise OriginalExceptionException.new("Second Exception", Exception.new("First Exception")) + }) + } + + it "shows the original exception instead of the last-raised one" do + expect(status).to eq(500) + expect(body).not_to match(/Second Exception/) + expect(body).to match(/First Exception/) + end end - it "won't crash if the exception responds_to but doesn't have an original_exception" do - app = Middleware.new(->env { - raise OriginalExceptionException.new("Other Exception") - }) - - status, _, body = app.call({}) + context 'and does not have one' do + let(:app) { + Middleware.new(->env { + raise OriginalExceptionException.new("The Exception") + }) + } - status.should == 500 - body.join.should match(/Other Exception/) + it "shows the exception as-is" do + expect(status).to eq(500) + expect(body).to match(/The Exception/) + end end end it "returns ExceptionWrapper's status_code" do ad_ew = double("ActionDispatch::ExceptionWrapper") - ad_ew.stub('new').with({}, exception ){ double("ExceptionWrapper", status_code: 404) } + allow(ad_ew).to receive('new').with(anything, exception) { double("ExceptionWrapper", status_code: 404) } stub_const('ActionDispatch::ExceptionWrapper', ad_ew) - status, headers, body = app.call({}) - - status.should == 404 + expect(status).to eq(404) end it "returns UTF-8 error pages" do - status, headers, body = app.call({}) + expect(headers["Content-Type"]).to match /charset=utf-8/ + end + + it "returns text content by default" do + expect(headers["Content-Type"]).to match /text\/plain/ + end + + context 'when a CSRF token cookie is not specified' do + it 'includes a newly-generated CSRF token cookie' do + expect(headers).to include( + 'Set-Cookie' => /BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=[-a-z0-9]+; path=\/; HttpOnly; SameSite=Strict/, + ) + end + end + + context 'when a CSRF token cookie is specified' do + let(:response_env) { app.call({ 'HTTP_COOKIE' => "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=abc123" }) } + + it 'does not set a new CSRF token cookie' do + expect(headers).not_to include('Set-Cookie') + end + end + + context 'when the Accept header specifies HTML first' do + let(:response_env) { app.call("HTTP_ACCEPT" => "text/html,application/xhtml+xml;q=0.9,*/*") } + + it "returns HTML content" do + expect(headers["Content-Type"]).to match /text\/html/ + end + + it 'includes the newly-generated CSRF token in the body of the page' do + matches = headers['Set-Cookie'].match(/BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=(?[-a-z0-9]+); path=\/; HttpOnly; SameSite=Strict/) + expect(body).to include(matches[:tok]) + end + + context 'when a CSRF token cookie is specified' do + let(:response_env) { + app.call({ + 'HTTP_COOKIE' => "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=csrfTokenGHI", + "HTTP_ACCEPT" => "text/html,application/xhtml+xml;q=0.9,*/*", + }) + } + + it 'includes that CSRF token in the body of the page' do + expect(body).to include('csrfTokenGHI') + end + end + end - headers["Content-Type"].should match /charset=utf-8/ + context 'the logger' do + let(:logger) { double('logger', fatal: nil) } + before do + allow(BetterErrors).to receive(:logger).and_return(logger) + end + + it "receives the exception as a fatal message" do + expect(logger).to receive(:fatal).with(/RuntimeError/) + response_env + end + + context 'when Rails is being used' do + before do + skip("Rails not included in this run") unless defined? Rails + end + + it "receives the exception without filtered backtrace frames" do + expect(logger).to receive(:fatal) do |message| + expect(message).to_not match(/rspec-core/) + end + response_env + end + end + context 'when Rails is not being used' do + before do + skip("Rails is included in this run") if defined? Rails + end + + it "receives the exception with all backtrace frames" do + expect(logger).to receive(:fatal) do |message| + expect(message).to match(/rspec-core/) + end + response_env + end + end end + end + + context "requesting the variables for a specific frame" do + let(:env) { {} } + let(:response_env) { + app.call(request_env) + } + let(:request_env) { + Rack::MockRequest.env_for("/__better_errors/#{id}/variables", input: StringIO.new(JSON.dump(request_body_data))) + } + let(:request_body_data) { { "index" => 0 } } + let(:json_body) { JSON.parse(body) } + let(:id) { 'abcdefg' } + + context 'when no errors have been recorded' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'No exception information available', + 'explanation' => /application has been restarted/, + ) + end + + context 'when Middleman is in use' do + let!(:middleman) { class_double("Middleman").as_stubbed_const } + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/Middleman reloads all dependencies/) + end + end + + context 'when Shotgun is in use' do + let!(:shotgun) { class_double("Shotgun").as_stubbed_const } + + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/The shotgun gem/) + end + + context 'when Hanami is also in use' do + let!(:hanami) { class_double("Hanami").as_stubbed_const } + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/--no-code-reloading/) + end + end + end + end + + context 'when an error has been recorded' do + let(:error_page) { ErrorPage.new(exception, env) } + before do + app.instance_variable_set('@error_page', error_page) + end + + context 'but it does not match the request' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Session expired', + 'explanation' => /no longer available in memory/, + ) + end + end + + context 'and its ID matches the requested ID' do + let(:id) { error_page.id } + + context 'when the body csrfToken matches the CSRF token cookie' do + let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } } + before do + request_env["HTTP_COOKIE"] = "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=csrfToken123" + end + + context 'when the Content-Type of the request is application/json' do + before do + request_env['CONTENT_TYPE'] = 'application/json' + end + + it 'returns JSON containing the HTML content' do + expect(error_page).to receive(:do_variables).and_return(html: "") + expect(json_body).to match( + 'html' => '', + ) + end + end + + context 'when the Content-Type of the request is application/json' do + before do + request_env['HTTP_CONTENT_TYPE'] = 'application/json' + end + + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Request not acceptable', + 'explanation' => /did not match an acceptable content type/, + ) + end + end + end - it "returns text pages by default" do - status, headers, body = app.call({}) + context 'when the body csrfToken does not match the CSRF token cookie' do + let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } } + before do + request_env["HTTP_COOKIE"] = "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=csrfToken456" + end + + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Invalid CSRF Token', + 'explanation' => /session might have been cleared/, + ) + end + end - headers["Content-Type"].should match /text\/plain/ + context 'when there is no CSRF token in the request' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Invalid CSRF Token', + 'explanation' => /session might have been cleared/, + ) + end + end + end end + end - it "returns HTML pages by default" do - # Chrome's 'Accept' header looks similar this. - status, headers, body = app.call("HTTP_ACCEPT" => "text/html,application/xhtml+xml;q=0.9,*/*") + context "requesting eval for a specific frame" do + let(:env) { {} } + let(:response_env) { + app.call(request_env) + } + let(:request_env) { + Rack::MockRequest.env_for("/__better_errors/#{id}/eval", input: StringIO.new(JSON.dump(request_body_data))) + } + let(:request_body_data) { { "index" => 0, source: "do_a_thing" } } + let(:json_body) { JSON.parse(body) } + let(:id) { 'abcdefg' } + + context 'when no errors have been recorded' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'No exception information available', + 'explanation' => /application has been restarted/, + ) + end - headers["Content-Type"].should match /text\/html/ + context 'when Middleman is in use' do + let!(:middleman) { class_double("Middleman").as_stubbed_const } + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/Middleman reloads all dependencies/) + end + end + + context 'when Shotgun is in use' do + let!(:shotgun) { class_double("Shotgun").as_stubbed_const } + + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/The shotgun gem/) + end + + context 'when Hanami is also in use' do + let!(:hanami) { class_double("Hanami").as_stubbed_const } + it 'returns a JSON error' do + expect(json_body['explanation']) + .to match(/--no-code-reloading/) + end + end + end end - it "logs the exception" do - logger = Object.new - logger.should_receive :fatal - BetterErrors.stub(:logger).and_return(logger) + context 'when an error has been recorded' do + let(:error_page) { ErrorPage.new(exception, env) } + before do + app.instance_variable_set('@error_page', error_page) + end + + context 'but it does not match the request' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Session expired', + 'explanation' => /no longer available in memory/, + ) + end + end + + context 'and its ID matches the requested ID' do + let(:id) { error_page.id } + + context 'when the body csrfToken matches the CSRF token cookie' do + let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } } + before do + request_env["HTTP_COOKIE"] = "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=csrfToken123" + end + + context 'when the Content-Type of the request is application/json' do + before do + request_env['CONTENT_TYPE'] = 'application/json' + end + + it 'returns JSON containing the eval result' do + expect(error_page).to receive(:do_eval).and_return(prompt: '#', result: "much_stuff_here") + expect(json_body).to match( + 'prompt' => '#', + 'result' => 'much_stuff_here', + ) + end + end + + context 'when the Content-Type of the request is application/json' do + before do + request_env['HTTP_CONTENT_TYPE'] = 'application/json' + end + + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Request not acceptable', + 'explanation' => /did not match an acceptable content type/, + ) + end + end + end + + context 'when the body csrfToken does not match the CSRF token cookie' do + let(:request_body_data) { { "index" => 0, "csrfToken" => "csrfToken123" } } + before do + request_env["HTTP_COOKIE"] = "BetterErrors-#{BetterErrors::VERSION}-CSRF-Token=csrfToken456" + end + + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Invalid CSRF Token', + 'explanation' => /session might have been cleared/, + ) + end + end + + context 'when there is no CSRF token in the request' do + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Invalid CSRF Token', + 'explanation' => /session might have been cleared/, + ) + end + end + end + end + end - app.call({}) + context "requesting an invalid internal method" do + let(:env) { {} } + let(:response_env) { + app.call(request_env) + } + let(:request_env) { + Rack::MockRequest.env_for("/__better_errors/#{id}/invalid", input: StringIO.new(JSON.dump(request_body_data))) + } + let(:request_body_data) { { "index" => 0 } } + let(:json_body) { JSON.parse(body) } + let(:id) { 'abcdefg' } + + it 'returns a JSON error' do + expect(json_body).to match( + 'error' => 'Not found', + 'explanation' => /recognized internal call/, + ) end end end diff --git a/spec/better_errors/raised_exception_spec.rb b/spec/better_errors/raised_exception_spec.rb index 6ffe86b1..45fa730d 100644 --- a/spec/better_errors/raised_exception_spec.rb +++ b/spec/better_errors/raised_exception_spec.rb @@ -1,31 +1,52 @@ require "spec_helper" +require "rspec/its" module BetterErrors describe RaisedException do let(:exception) { RuntimeError.new("whoops") } subject { RaisedException.new(exception) } - its(:exception) { should == exception } - its(:message) { should == "whoops" } - its(:type) { should == RuntimeError } + its(:exception) { is_expected.to eq exception } + its(:message) { is_expected.to eq "whoops" } + its(:type) { is_expected.to eq RuntimeError } - context "when the exception wraps another exception" do + context 'when the exception is an ActionView::Template::Error that responds to #cause (Rails 6+)' do + before do + stub_const( + "ActionView::Template::Error", + Class.new(StandardError) do + def cause + RuntimeError.new("something went wrong!") + end + end + ) + end + let(:exception) { + ActionView::Template::Error.new("undefined method `something!' for #") + } + + its(:message) { is_expected.to eq "something went wrong!" } + its(:type) { is_expected.to eq RuntimeError } + end + + context 'when the exception is a Rails < 6 exception that has an #original_exception' do let(:original_exception) { RuntimeError.new("something went wrong!") } let(:exception) { double(:original_exception => original_exception) } - its(:exception) { should == original_exception } - its(:message) { should == "something went wrong!" } + its(:exception) { is_expected.to eq original_exception } + its(:message) { is_expected.to eq "something went wrong!" } + its(:type) { is_expected.to eq RuntimeError } end - context "when the exception is a syntax error" do + context "when the exception is a SyntaxError" do let(:exception) { SyntaxError.new("foo.rb:123: you made a typo!") } - its(:message) { should == "you made a typo!" } - its(:type) { should == SyntaxError } + its(:message) { is_expected.to eq "you made a typo!" } + its(:type) { is_expected.to eq SyntaxError } it "has the right filename and line number in the backtrace" do - subject.backtrace.first.filename.should == "foo.rb" - subject.backtrace.first.line.should == 123 + expect(subject.backtrace.first.filename).to eq("foo.rb") + expect(subject.backtrace.first.line).to eq(123) end end @@ -40,15 +61,29 @@ module BetterErrors end } - its(:message) { should == "you made a typo!" } - its(:type) { should == Haml::SyntaxError } + its(:message) { is_expected.to eq "you made a typo!" } + its(:type) { is_expected.to eq Haml::SyntaxError } it "has the right filename and line number in the backtrace" do - subject.backtrace.first.filename.should == "foo.rb" - subject.backtrace.first.line.should == 123 + expect(subject.backtrace.first.filename).to eq("foo.rb") + expect(subject.backtrace.first.line).to eq(123) end end + # context "when the exception is an ActionView::Template::Error" do + # + # let(:exception) { + # ActionView::Template::Error.new("undefined method `something!' for #") + # } + # + # its(:message) { is_expected.to eq "undefined method `something!' for #" } + # + # it "has the right filename and line number in the backtrace" do + # expect(subject.backtrace.first.filename).to eq("app/views/foo/bar.haml") + # expect(subject.backtrace.first.line).to eq(42) + # end + # end + # context "when the exception is a Coffeelint syntax error" do before do stub_const("Sprockets::Coffeelint::Error", Class.new(SyntaxError)) @@ -60,12 +95,12 @@ module BetterErrors end } - its(:message) { should == "[stdin]:11:88: error: unexpected=" } - its(:type) { should == Sprockets::Coffeelint::Error } + its(:message) { is_expected.to eq "[stdin]:11:88: error: unexpected=" } + its(:type) { is_expected.to eq Sprockets::Coffeelint::Error } it "has the right filename and line number in the backtrace" do - subject.backtrace.first.filename.should == "app/assets/javascripts/files/index.coffee" - subject.backtrace.first.line.should == 11 + expect(subject.backtrace.first.filename).to eq("app/assets/javascripts/files/index.coffee") + expect(subject.backtrace.first.line).to eq(11) end end diff --git a/spec/better_errors/repl/basic_spec.rb b/spec/better_errors/repl/basic_spec.rb index cc71a955..1db8b4c9 100644 --- a/spec/better_errors/repl/basic_spec.rb +++ b/spec/better_errors/repl/basic_spec.rb @@ -10,7 +10,9 @@ module REPL binding } - let(:repl) { Basic.new fresh_binding } + let!(:exception) { raise ZeroDivisionError, "you divided by zero you silly goose!" rescue $! } + + let(:repl) { Basic.new(fresh_binding, exception) } it_behaves_like "a REPL provider" end diff --git a/spec/better_errors/repl/pry_spec.rb b/spec/better_errors/repl/pry_spec.rb index 1aa502c5..984f03b5 100644 --- a/spec/better_errors/repl/pry_spec.rb +++ b/spec/better_errors/repl/pry_spec.rb @@ -1,40 +1,51 @@ require "spec_helper" -require "pry" -require "better_errors/repl/pry" require "better_errors/repl/shared_examples" -module BetterErrors - module REPL - describe Pry do - let(:fresh_binding) { - local_a = 123 - binding - } - - let(:repl) { Pry.new fresh_binding } - - it "does line continuation" do - output, prompt, filled = repl.send_input "" - output.should == "=> nil\n" - prompt.should == ">>" - filled.should == "" - - output, prompt, filled = repl.send_input "def f(x)" - output.should == "" - prompt.should == ".." - filled.should == " " - - output, prompt, filled = repl.send_input "end" - if RUBY_VERSION >= "2.1.0" - output.should == "=> :f\n" - else - output.should == "=> nil\n" - end - prompt.should == ">>" - filled.should == "" - end +if defined? ::Pry + RSpec.describe 'BetterErrors::REPL::Pry' do + before(:all) do + load "better_errors/repl/pry.rb" + end + after(:all) do + # Ensure the Pry REPL file has not been included. If this is not done, + # the constant leaks into other examples. + # In practice, this constant is only defined if `use_pry!` is called and then the + # REPL is used, causing BetterErrors::REPL to require the file. + BetterErrors::REPL.send(:remove_const, :Pry) + end + + let(:fresh_binding) { + local_a = 123 + binding + } + + let!(:exception) { raise ZeroDivisionError, "you divided by zero you silly goose!" rescue $! } - it_behaves_like "a REPL provider" + let(:repl) { BetterErrors::REPL::Pry.new(fresh_binding, exception) } + + it "does line continuation", :aggregate_failures do + output, prompt, filled = repl.send_input "" + expect(output).to eq("=> nil\n") + expect(prompt).to eq(">>") + expect(filled).to eq("") + + output, prompt, filled = repl.send_input "def f(x)" + expect(output).to eq("") + expect(prompt).to eq("..") + expect(filled).to eq(" ") + + output, prompt, filled = repl.send_input "end" + if RUBY_VERSION >= "2.1.0" + expect(output).to eq("=> :f\n") + else + expect(output).to eq("=> nil\n") + end + expect(prompt).to eq(">>") + expect(filled).to eq("") end + + it_behaves_like "a REPL provider" end +else + puts "Skipping Pry specs because pry is not in the bundle" end diff --git a/spec/better_errors/repl/shared_examples.rb b/spec/better_errors/repl/shared_examples.rb index 4925d8f1..296c9f29 100644 --- a/spec/better_errors/repl/shared_examples.rb +++ b/spec/better_errors/repl/shared_examples.rb @@ -1,18 +1,18 @@ shared_examples_for "a REPL provider" do it "evaluates ruby code in a given context" do repl.send_input("local_a = 456") - fresh_binding.eval("local_a").should == 456 + expect(fresh_binding.eval("local_a")).to eq(456) end it "returns a tuple of output and the new prompt" do output, prompt = repl.send_input("1 + 2") - output.should == "=> 3\n" - prompt.should == ">>" + expect(output).to eq("=> 3\n") + expect(prompt).to eq(">>") end it "doesn't barf if the code throws an exception" do output, prompt = repl.send_input("raise Exception") - output.should include "Exception: Exception" - prompt.should == ">>" + expect(output).to include "Exception: Exception" + expect(prompt).to eq(">>") end end diff --git a/spec/better_errors/stack_frame_spec.rb b/spec/better_errors/stack_frame_spec.rb index 3d9505c0..df96c90a 100644 --- a/spec/better_errors/stack_frame_spec.rb +++ b/spec/better_errors/stack_frame_spec.rb @@ -4,80 +4,100 @@ module BetterErrors describe StackFrame do context "#application?" do it "is true for application filenames" do - BetterErrors.stub(:application_root).and_return("/abc/xyz") + allow(BetterErrors).to receive(:application_root).and_return("/abc/xyz") frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") - frame.should be_application + expect(frame).to be_application end it "is false for everything else" do - BetterErrors.stub(:application_root).and_return("/abc/xyz") + allow(BetterErrors).to receive(:application_root).and_return("/abc/xyz") frame = StackFrame.new("/abc/nope", 123, "foo") - frame.should_not be_application + expect(frame).not_to be_application end it "doesn't care if no application_root is set" do frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") - frame.should_not be_application + expect(frame).not_to be_application end end context "#gem?" do it "is true for gem filenames" do - Gem.stub(:path).and_return(["/abc/xyz"]) + allow(Gem).to receive(:path).and_return(["/abc/xyz"]) frame = StackFrame.new("/abc/xyz/gems/whatever-1.2.3/lib/whatever.rb", 123, "foo") - frame.should be_gem + expect(frame).to be_gem end it "is false for everything else" do - Gem.stub(:path).and_return(["/abc/xyz"]) + allow(Gem).to receive(:path).and_return(["/abc/xyz"]) frame = StackFrame.new("/abc/nope", 123, "foo") - frame.should_not be_gem + expect(frame).not_to be_gem end end context "#application_path" do it "chops off the application root" do - BetterErrors.stub(:application_root).and_return("/abc/xyz") + allow(BetterErrors).to receive(:application_root).and_return("/abc/xyz") frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") - frame.application_path.should == "app/controllers/crap_controller.rb" + expect(frame.application_path).to eq("app/controllers/crap_controller.rb") end end context "#gem_path" do it "chops of the gem path and stick (gem) there" do - Gem.stub(:path).and_return(["/abc/xyz"]) + allow(Gem).to receive(:path).and_return(["/abc/xyz"]) frame = StackFrame.new("/abc/xyz/gems/whatever-1.2.3/lib/whatever.rb", 123, "foo") - frame.gem_path.should == "whatever (1.2.3) lib/whatever.rb" + expect(frame.gem_path).to eq("whatever (1.2.3) lib/whatever.rb") end it "prioritizes gem path over application path" do - BetterErrors.stub(:application_root).and_return("/abc/xyz") - Gem.stub(:path).and_return(["/abc/xyz/vendor"]) + allow(BetterErrors).to receive(:application_root).and_return("/abc/xyz") + allow(Gem).to receive(:path).and_return(["/abc/xyz/vendor"]) frame = StackFrame.new("/abc/xyz/vendor/gems/whatever-1.2.3/lib/whatever.rb", 123, "foo") - frame.gem_path.should == "whatever (1.2.3) lib/whatever.rb" + expect(frame.gem_path).to eq("whatever (1.2.3) lib/whatever.rb") end end context "#pretty_path" do it "returns #application_path for application paths" do - BetterErrors.stub(:application_root).and_return("/abc/xyz") + allow(BetterErrors).to receive(:application_root).and_return("/abc/xyz") frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") - frame.pretty_path.should == frame.application_path + expect(frame.pretty_path).to eq(frame.application_path) end it "returns #gem_path for gem paths" do - Gem.stub(:path).and_return(["/abc/xyz"]) + allow(Gem).to receive(:path).and_return(["/abc/xyz"]) frame = StackFrame.new("/abc/xyz/gems/whatever-1.2.3/lib/whatever.rb", 123, "foo") - frame.pretty_path.should == frame.gem_path + expect(frame.pretty_path).to eq(frame.gem_path) + end + end + + context "#local_variable" do + it "returns exception details when #get_local_variable raises NameError" do + frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") + allow(frame).to receive(:get_local_variable).and_raise(NameError.new("details")) + expect(frame.local_variable("foo")).to eq("NameError: details") + end + + it "returns exception details when #eval_local_variable raises NameError" do + frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") + allow(frame).to receive(:eval_local_variable).and_raise(NameError.new("details")) + expect(frame.local_variable("foo")).to eq("NameError: details") + end + + it "raises on non-NameErrors" do + frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index") + allow(frame).to receive(:get_local_variable).and_raise(ArgumentError) + expect { frame.local_variable("foo") }.to raise_error(ArgumentError) end end @@ -87,36 +107,36 @@ module BetterErrors rescue SyntaxError => syntax_error end frames = StackFrame.from_exception(syntax_error) - frames.first.filename.should == "my_file.rb" - frames.first.line.should == 123 + expect(frames.first.filename).to eq("my_file.rb") + expect(frames.first.line).to eq(123) end it "doesn't blow up if no method name is given" do error = StandardError.allocate - error.stub(:backtrace).and_return(["foo.rb:123"]) + allow(error).to receive(:backtrace).and_return(["foo.rb:123"]) frames = StackFrame.from_exception(error) - frames.first.filename.should == "foo.rb" - frames.first.line.should == 123 + expect(frames.first.filename).to eq("foo.rb") + expect(frames.first.line).to eq(123) - error.stub(:backtrace).and_return(["foo.rb:123: this is an error message"]) + allow(error).to receive(:backtrace).and_return(["foo.rb:123: this is an error message"]) frames = StackFrame.from_exception(error) - frames.first.filename.should == "foo.rb" - frames.first.line.should == 123 + expect(frames.first.filename).to eq("foo.rb") + expect(frames.first.line).to eq(123) end it "ignores a backtrace line if its format doesn't make any sense at all" do error = StandardError.allocate - error.stub(:backtrace).and_return(["foo.rb:123:in `foo'", "C:in `find'", "bar.rb:123:in `bar'"]) + allow(error).to receive(:backtrace).and_return(["foo.rb:123:in `foo'", "C:in `find'", "bar.rb:123:in `bar'"]) frames = StackFrame.from_exception(error) - frames.count.should == 2 + expect(frames.count).to eq(2) end it "doesn't blow up if a filename contains a colon" do error = StandardError.allocate - error.stub(:backtrace).and_return(["crap:filename.rb:123"]) + allow(error).to receive(:backtrace).and_return(["crap:filename.rb:123"]) frames = StackFrame.from_exception(error) - frames.first.filename.should == "crap:filename.rb" + expect(frames.first.filename).to eq("crap:filename.rb") end it "doesn't blow up with a BasicObject as frame binding" do @@ -125,7 +145,7 @@ def obj.my_binding ::Kernel.binding end frame = StackFrame.new("/abc/xyz/app/controllers/crap_controller.rb", 123, "index", obj.my_binding) - frame.class_name.should == 'BasicObject' + expect(frame.class_name).to eq('BasicObject') end it "sets method names properly" do @@ -140,11 +160,11 @@ def obj.my_method frame = StackFrame.from_exception(obj.my_method).first if BetterErrors.binding_of_caller_available? - frame.method_name.should == "#my_method" - frame.class_name.should == "String" + expect(frame.method_name).to eq("#my_method") + expect(frame.class_name).to eq("String") else - frame.method_name.should == "my_method" - frame.class_name.should == nil + expect(frame.method_name).to eq("my_method") + expect(frame.class_name).to eq(nil) end end diff --git a/spec/better_errors_spec.rb b/spec/better_errors_spec.rb index e9b26b67..9d796105 100644 --- a/spec/better_errors_spec.rb +++ b/spec/better_errors_spec.rb @@ -3,38 +3,45 @@ describe BetterErrors do context ".editor" do it "defaults to textmate" do - subject.editor["foo.rb", 123].should == "txmt://open?url=file://foo.rb&line=123" + expect(subject.editor["foo.rb", 123]).to eq("txmt://open?url=file://foo.rb&line=123") end it "url escapes the filename" do - subject.editor["&.rb", 0].should == "txmt://open?url=file://%26.rb&line=0" + expect(subject.editor["&.rb", 0]).to eq("txmt://open?url=file://%26.rb&line=0") end [:emacs, :emacsclient].each do |editor| it "uses emacs:// scheme when set to #{editor.inspect}" do subject.editor = editor - subject.editor[].should start_with "emacs://" + expect(subject.editor[]).to start_with "emacs://" end end [:macvim, :mvim].each do |editor| it "uses mvim:// scheme when set to #{editor.inspect}" do subject.editor = editor - subject.editor[].should start_with "mvim://" + expect(subject.editor[]).to start_with "mvim://" end end [:sublime, :subl, :st].each do |editor| it "uses subl:// scheme when set to #{editor.inspect}" do subject.editor = editor - subject.editor[].should start_with "subl://" + expect(subject.editor[]).to start_with "subl://" end end [:textmate, :txmt, :tm].each do |editor| it "uses txmt:// scheme when set to #{editor.inspect}" do subject.editor = editor - subject.editor[].should start_with "txmt://" + expect(subject.editor[]).to start_with "txmt://" + end + end + + [:atom].each do |editor| + it "uses atom:// scheme when set to #{editor.inspect}" do + subject.editor = editor + expect(subject.editor[]).to start_with "atom://" end end @@ -42,7 +49,7 @@ it "uses emacs:// scheme when EDITOR=#{editor}" do ENV["EDITOR"] = editor subject.editor = subject.default_editor - subject.editor[].should start_with "emacs://" + expect(subject.editor[]).to start_with "emacs://" end end @@ -50,15 +57,15 @@ it "uses mvim:// scheme when EDITOR=#{editor}" do ENV["EDITOR"] = editor subject.editor = subject.default_editor - subject.editor[].should start_with "mvim://" + expect(subject.editor[]).to start_with "mvim://" end end ["subl -w", "/Applications/Sublime Text 2.app/Contents/SharedSupport/bin/subl"].each do |editor| - it "uses mvim:// scheme when EDITOR=#{editor}" do + it "uses subl:// scheme when EDITOR=#{editor}" do ENV["EDITOR"] = editor subject.editor = subject.default_editor - subject.editor[].should start_with "subl://" + expect(subject.editor[]).to start_with "subl://" end end @@ -66,7 +73,40 @@ it "uses txmt:// scheme when EDITOR=#{editor}" do ENV["EDITOR"] = editor subject.editor = subject.default_editor - subject.editor[].should start_with "txmt://" + expect(subject.editor[]).to start_with "txmt://" + end + end + + + ["atom -w", "/usr/bin/atom -w"].each do |editor| + it "uses atom:// scheme when EDITOR=#{editor}" do + ENV["EDITOR"] = editor + subject.editor = subject.default_editor + expect(subject.editor[]).to start_with "atom://" + end + end + + ["mine"].each do |editor| + it "uses x-mine:// scheme when EDITOR=#{editor}" do + ENV["EDITOR"] = editor + subject.editor = subject.default_editor + expect(subject.editor[]).to start_with "x-mine://" + end + end + + ["idea"].each do |editor| + it "uses idea:// scheme when EDITOR=#{editor}" do + ENV["EDITOR"] = editor + subject.editor = subject.default_editor + expect(subject.editor[]).to start_with "idea://" + end + end + + ["vscode", "code"].each do |editor| + it "uses vscode:// scheme when EDITOR=#{editor}" do + ENV["EDITOR"] = editor + subject.editor = subject.default_editor + expect(subject.editor[]).to start_with "vscode://" end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 40d63261..29e5ac18 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,4 +2,26 @@ ENV["EDITOR"] = nil -require "better_errors" +# Ruby 2.4.0 and 2.4.1 has a bug with its Coverage module that causes segfaults. +# https://bugs.ruby-lang.org/issues/13305 +# 2.4.2 should include this patch. +if ENV['CI'] + unless RUBY_VERSION == '2.4.0' || RUBY_VERSION == '2.4.1' + require 'coveralls' + Coveralls.wear! do + add_filter 'spec/' + end + end +else + require 'simplecov' + SimpleCov.start +end + +require 'bundler/setup' +Bundler.require(:default) + +require 'rspec-html-matchers' + +RSpec.configure do |config| + config.include RSpecHtmlMatchers +end