CSC/ECE 517 Fall 2017/E1773 Investigate and Fix Expertiza Production Version Runtime Exceptions.rb
Introduction
Background
Expertizais a open-source ruby-on-rails project on github. It constructs a peer-review system by enabling interactions among users and instructors. Students can sign up for a class, view assignments, submit assignments and give peer reviews using expertiza. Instructors can publish assignments, surveys, and reviews, view statistical results and make announcements. The website is created and currently maintained mainly by the students and faculties from NCSU.
Motivation
Airbrake is a online debugging tool for rails projects that is currently incorporated in Expertiza. Thus, Expertiza production errors at run time are tracked and reported statistically to Airbrake for reviewing and debugging. Our goal for this OSS project is to fix the top 10 run time exceptions ranked by occurrences reported to Airbrake. You can check the 10 most ranked exceptions by click on the previous hyperlink.
Airbrake Run-time Exceptions and Fixes
By investigating into the top 10 errors from airbrake, we can divide them into different categories by the root of their causes.
1. NoMethodError: undefined method for nil:NilClass (4 out of 10)
2. AbstractController::ActionNotFound (4 out of 10)
3. ActionController::InvalidAuthenticityToken (1 out of 10)
4. ActiveRecord::RecordNotFound: Couldn't find Participant without an ID (1 out of 10)
As we have discovered, the same or similar fix can be used to fix all exceptions in the same different category, which is how we categorized these exceptions initially.
Files Modified
/config/routes.rb
Exception 1. NoMethodError: undefined method 'call' for nil:NilClass
Problem Description:
From the Airbrake issue page, we could find out that the problem is actually caused by a gem called passenger.
Although we couldn't find any sign of passenger gem in the recent master branch, but there is a clear sign we
found during our review of other errors that some developers had used passenger.
The gem might be under the production path `/home/rails/.rvm/gems/ruby-2.1.5/gems/passenger-5.0.16/'
So the solution might be just to remove the gem passenger from production environment.
For more information, please check here
NoMethodError: undefined method 'role' for nil:NilClass is one similar error that occurs during run-time
Solution
After investigating this error by going to the error causing url,as shown in pic1, given by airbrake under development environment, http://localhost:3000/response/view?id=91251, the error is verified does not appear again. The last time this error occurred was 5 months ago. Note, the link is only accessible to super admin account. As you may have noticed, the picture on the right shows the passenger gem is called and traced, which we couldn't find such a gem installed in the current gemfile.
NoMethodError: undefined method 'uniq' for nil:NilClass is one similar error that occurs during run-time
Similarly, by going to the error cause url(https://expertiza.ncsu.edu/sign_up_sheet/set_priority) again under development environment, there is no such page exist.
Exception 2. AbstractController::ActionNotFound
Problem Description
In most of the cases, the error is caused by an abusive use of RESTful routing provided in rails. In the middle of somewhere of the app, an index or show action is considered "called" when the redirect url matches such an action which is undefined in the controller.
By thoroughly investigating the controller in trouble and its routes, we came up with a solution that to either reroute the url to an effective url or abort the RESTful route.
For one example, in app/controllers/auth_controller, we found no show/index action defined or needed, but in /config/routes.rb, we have the following:
=begin
resources :auth , :except=>[:show,:index,:create,:destroy,:update,:edit,:new] do
collection do
post :login
post :logout
end
end
=end
Solution
By using resources:auth, rails actually creates index and show actions by default. Somewhere in the app, a redirect is made available to the user to an index or show view page which does not exist. Thus, in order to save the trouble finding the redirect, we simply redirect anything points to those invalid pages to a valid page, which is the root.The modified code is below:
post '/auth/login' ,to:"auth#login"
post '/auth/logout' ,to:"auth#logout"
get '/auth/*path', to: redirect('/')
Following the same principle, we have modified /config/routes.rb for routes that have caused the most exceptions due to exposure of invalid url redirection to the user.
resources :response, :except =>[:index]do
collection do
get :new_feedback
get :view
post :delete
get :remove_hyperlink
get :saving
get :redirection
get :show_calibration_results_for_student
post :custom_create
get :pending_surveys
get '/*path', to: redirect('/response/pending_surveys')
end
end
There are 10 similar errors in Top 40 errors involved 5 controllers, which are ResponseController, AuthController, AdminController, ImpersonateController and PublishingController. If a controller mentioned above never uses a method declared by resources, we delete the relevant resources and add the routes of method separately, otherwise we use except expression to exclude methods we don't use.
Exception 3. ActionController::InvalidAuthenticityToken
Problem
Error Message: You submitted a form with an outdated or missing authenticity token. Try reloading the page you just submitted and submit the form again.
The error was raised in config/initializers/request_forgery_protection_mod.rb
And the error actually happened in response#create
The background of this problem is that The authenticity token is a random value generated in your view to prove a request is submitted from a form on your site, not somewhere else. This protects against CSRF attacks.
Solution
The expertiza problem could be the user session expired and the solution can be delete the code in the config/initializers/request_forgery_protection_mod.rb, and redirect or just refresh the page, which would not raise an error. This error is not necessarily a bug in the code, but rather a protection masochism. As said before, it is a matter of choice to get rid of this error or not. Raising an exception is the simplest way to stop the app when some behavior is not wanted.
Exception 4. RuntimeError: This topic has too many reviews; please select another one
Problem
The error was raised in app/models/review_assignment.rb:171
And the error actually happened in review_mapping#assign_reviewer_dynamically
The intuition is that the review to a topic should be assigned dynamically and "too many reviews" problem should be handled by action revew_mapping#assign_reviewer_dynamically. So we could add the below condition in the above controller action to handle this problem.
Solution
Instead of raising an error, we could simply handle this problem, which can make "flash danger message" unnecessary. Also, the solution is a matter of choice. The simplest way to stop users to submit or post any new data into the data base is to raise an exception which will stop the app. So we are merely making a suggestion here.
Test Plan
Running the project Locally
First, follow the instructions given on Expertiza. It's your choice to use virtual machine or set up your own environment. Clone the repository which is also the submitted Pull request. In this project, we will mainly use airbrake to do testing.
Setting up Airbrake
Expertiza has already implemented the airbrake gem. You need an airbrake account to begin test. In /config/initializers/airbrake.rb , change the following line
c.ignore_environments = %w(development test) => c.ignore_environments = %w(test)
c.project_id = 'your airbrake account id'
c.project_key = 'your airbrake project_key'
You will get your project id after you registered an airbrake account and created a new project on it. It is in your account profile. where project key will be at the same location with your project id.
Once you have the right key and id pair, you can start testing the run-time exceptions.