Friday, April 17, 2009

Dealing with Errors in Rails

Jeff Atwood's recent blog post on "Exception-Driven Development" has driven me to write a post that I've been thinking about writing for the last couple weeks.

I'm working on a Rails application in my spare time, and one of the concepts I wanted to drill into the codebase from the beginning was easy management of the system from within the system. This means I can log in and, given that I have the proper access, view errors that have happened in the site, migrate the database, restart the application, and perform various other actions that would otherwise require tedious activities like digging in log files, or obtaining shell access and executing various commands. While I am quite comfortable on the shell (I run Linux on all the machines that I use with any regularity), I would much rather have an easy to use GUI that gives me access in the context of when I need it (ie, when I am actually checking up on the website... from within it).

So, this post will basically show you how to add error logging within your Rails application, as I've done in mine.

First up: the database. Here's the database migration script you will need:

class CreateErrors < ActiveRecord::Migration
def self.up
create_table :errors do |t|
t.string :level
t.string :title
t.boolean :handled
t.text :description

t.timestamps
end
end

def self.down
drop_table :errors
end
end


The level column is so you can keep track of errors vs warnings, or have informational messages as well. It's not strictly necessary, but I want the flexibility, plus I'm used to the concept from all the logging frameworks that support multiple levels of messages.

The title and description should be self explanatory. In case you are slow though, the title is just a header so you can at a glance see what errors have cropped up, and the description provides a detailed look at the error (such as a backtrace).

The handled column is critical so you can keep track of which errors you have dealt with, and which need looking at. It acts as a soft delete.

Since you've seen the database end, you might as well peak at the model next:

class Error < ActiveRecord::Base
def self.error(title, description)
log :error, title, description
end

def self.warn(title, description)
log :warn, title, description
end

def self.info(title, description)
log :info, title, description
end

def self.log(level, title, description)
Error.new(:level => level.to_s, :title => title,
:description => description, :handled => false).save
end
end


The error, warn and info methods are to easily log something at the given level, much like a typical logging library. All of those methods use the one logging method which will actually save the error as a row in the database. Pretty simple, as is the usual case with ActiveRecord classes! You could make a case for making the log method private, but I don't really want to, so I won't.

Next up, the errors controller:

class ErrorsController < ApplicationController
title "Errors"
require_admin

def index
@total = Error.count :all, :conditions => ["handled = ?", false]
@errors = Error.find :all,
:conditions => ["handled = ?", false],
:order => "created_at DESC", :limit => 50
@start = [1, @total].min
@amount = @errors.size
end

def show
@error = Error.find params[:id]
end

def handle
@error = Error.find params[:id]
@error.update_attributes :handled => true
redirect_to errors_url
end
end


The title and require_admin methods are some utility methods I baked into the base controller... I may publish those at some point, but they aren't the focus of this post, so not now. If I get any comments requesting the code, I will probably follow up with the code, so let me know if it is intriguing you! Their purpose are to set the title on all views associated with this controller, and check for administrator privileges respectively.

If you will notice, the index will only load the most recent 50 errors... I figured it would be possible for the application to get in a weird loop where you trigger tons and tons of errors. I wouldn't want to haplessly check out the current list of errors, only to watch in horror as I am downloading a list of 100,000 errors. The handle method is the only means to update an error... and it simple turns on the soft delete so the error won't show anymore. It might make sense to set up a mass handled action which would handle a group of selected errors all at once... but I don't really need that yet... I will build it when the need arises.

The rest in index should make sense as you check out the index.html.erb view:

<h1>Listing errors</h1>

<p>
<%= @start %> - <%= @amount %> of <%= @total %> errors.
</p>

<table>
<tr>
<th>Date</th>
<th>Level</th>
<th>Title</th>
</tr>

<% @errors.each do |error| %>
<tr>
<td><%= h format_datetime(error.created_at) %></td>
<td><%= h error.level %></td>
<td><%= h truncate(error.title, :length => 100) %></td>
<td><%= h error.handled %></td>
<td><%= link_to "Show", error %></td>
<td><a href="/errors/handle/<%= error.id %>">Handled</a></td>
</tr>
<% end %>
</table>


Pretty straightforward, huh? Well, here's show.html.erb

<p>
<b>Title:</b>
<%= h @error.title %>
</p>

<p>
<b>Time:</b>
<%= h format_datetime(@error.created_at) %>
</p>

<p>
<b>Level:</b>
<%= h @error.level %>
</p>

<p>
<b>Description:</b>

<br/>

<%= format_multilines h(@error.description) %>
</p>

<a href="/errors">Back</a>


Also pretty simple. If you are curious about format_datetime and format_multilines, they are simple helpers I created to respectively format a date as I want them formatted, and take a string with newlines and convert the newlines to <br/> elements. Pretty simple helpers, but I like keeping as much logic off my views as possible.

That's it! Or wait... that's all the stuff that rails practically generates for you (tweaked for simplicity, but still). How can this stuff get put into practice? Well, I'll give you that code too... since you've gotten this far. In your base application controller:

class ApplicationController < ActionController::Base
RENDER_LIMIT = 0.5
around_filter :time_page

...

private
def time_page
before = Time.new
yield
after = Time.new
delta = after - before

if delta >= RENDER_LIMIT
title = "Page took #{delta} seconds to render"
description = "Execution of #{request.path} took #{delta} seconds to render."
Error.warn title, description
end
end

def rescue_action(e)
title = "Caught exception '#{e}'"
description = "Exception was caught: '#{e}'\n\nBacktrace:\n#{e.backtrace.join("\n")}"
Error.error title, description
super
end
end


With this addition, your new in-site error reporting mechanism will report any page that takes over half a second to load, or absolutely any exception that crops up. This includes failing to parse a Ruby file, actions or controllers that were browsed to that don't exist, or any random exception the user encounters. You may want to adjust the RENDER_LIMIT constant to the threshold of your liking. At some point I would like to add a separate timer for all database queries, but I haven't had the time or interest to do it yet... and besides, the render limit should encompass all queries anyways, so it would be slightly redundant (though I would like to have a much lower threshold of query time than render time).

And as a caveat, this will not report a failure to parse the application controller Ruby file... because... well... think about it and you should realize why it can't :-)

Go forth and practice Exception-Driven Development in Rails.

Tuesday, April 14, 2009

jQuery UI Theme... A Tribute

Ok, I haven't blogged in a while, and I have been intending to start... with maybe little bite sized entries. I have the perfect one now, so here it is... my very own (and first custom) jQuery UI Theme.

In case you can't figure it out... I call it "Hot Dog Stand."

I am actually including it in a web app I am building.

Monday, October 13, 2008

Why Reddit is Better than Stack Overflow (WAAAAY Better)

Ok, so to start, I am a recovering former addict of crackoverfl.... I mean stackoverflow.com. Honest! Look at my rank on the site (I go by "Mike Stone"). As I write this, I am 6th on the list of users sorted by reputation... and just a week or so ago I was holding steady at 3rd for quite a while. As you read this, I may not even be on that first page anymore.

There were times when I would be constantly looking for questions I could answer well... and other times I attempted to answer anything at all (that didn't go so well usually).

As a reformed addict, I think I can safely say a few things about the site. Whether they hold sway for you, or whether you agree or not... frankly I don't care much... because if you disagree, you are one of THEM....

So here it goes... Reddit has 1 thing going for it that I don't think Jeff Atwood will ever have... trust in the cloud.

That's it.

That's all there is to it.

The thing is, this is a touchy subject. In case you don't know what I'm talking about, I am alluding to the "close question" feature that Jeff so dogmatically believes is helping steer the users into asking the "right" questions. Instead, it is giving some users a power trip, because after all, they know what is good for the site. So they go ahead and close questions that clearly don't belong. Except some people find value in those questions, and it has a negative effect on the asker if they are powerless to disagree and change the decision.

If you are in the "we need to steer users towards our opinions" camp... I'll tell you right now that I hate you. Don't take it personally... what I really mean is I hate your stance, but it's more fun to just say I hate you. I don't like the power trip you feel, and I don't like that you are forcing your opinion down on my throat what should and should not be open for discussion. Yeah, the D word... I know Jeff doesn't like discussion on his precious baby of a site, but sometimes I want to go to the site for discussion. It's fun to answer answerable questions, and it's also fun to discuss interesting arguable questions with intelligent people (such as my most popular question). I want to do both, and I want to see both types of questions thrive.

But... Closing Questions is Working!

This is the argument I've seen from Jeff Atwood. He says closing questions is working, and he links to questions that have been closed as so-called proof. We have exchanged some emails, and when he sends me the links, I look at them. Every single time I've looked at his examples of questions that deserved to be closed, it is the same damn thing. The questions are either duplicates, have a negative vote total, have been deleted (likely because of offensive count), or they are gray area questions that more often than not I would rather see them on the site.

Duplicates Should be Closed, Right?

This is the single best reason for having the close question feature, except I think it is a problem best solved with a different, unimplemented feature. At least half the questions I see closed are marked as duplicates. Wouldn't it be neat if instead of closing duplicates, all the duplicates were congealed into 1 massive blob of a question? You go to one, and you see the blob... and all the questions are available, able to morph between the different forms via tabs (or some other mechanism). This way, the answers to all these similar questions can be viewed at once without having to scan for links to follow (and if you can't provide a link to the duplicate in question, don't close it as a duplicate... that angers me more than the close feature in general). The oldest one could even always be the first tab, so there is 1 canonical place to get the best answers, and all the others turn into alternative forms with possible nuggets of gold awaiting you to click its tab.

I have a secret about duplicate questions though... Jeff actually likes keeping duplicates around! If you don't believe me, here's a quote from an email he sent me:

There's no value in most closed questions, with the exception of edited duplicates which allow better search matching. People have the uncanny ability to ask the exact same question using totally different words, so we need these as breadcrumb trails.

So if duplicates are an important part of the ecosystem, and it's the only good use of closed questions (which I hope to show before you stop reading), then closing questions shouldn't be necessary at all.....

This One is Negatively Voted... Isn't That Proof Closed Questions ARE Working?

Not exactly. If it's negatively voted, this seems perfect proof that the voting system was a good system to implement. If a question is negatively voted, then people should be able to watch "hot" tabs and just not even see the negatively voted questions. There will be plenty of people watching all new questions who will vote it up if it was voted down inappropriately, and those same people will continue to vote it down if it really should stay down. Thus... why does it need to be closed?

If someone finds value in a negatively voted question, what is the harm in keeping it around? If the asker gets an answer he/she is happy with, who loses? If it is tagged appropriately, it probably won't even show up in the tag filters people set up (or the site automatically figures out) so that people who would rather not see it, won't see it.

So those questions don't REALLY need to be closed... voting dealt with them.

I Saw a REALLY Offensive Question Closed... That's Good Right?

These questions get marked offensive, and they get automatically deleted after 5 offensive marks. That is the purpose of the offensive feature, and it deals with those questions well... closing them is like closing a negatively voted question. Saying it is bad twice doesn't make it extra bad... just 1 way to say it is bad is all that is really needed (well, 1 way to say it's bad, and one to say it's REALLY bad... hense downvotes and offensive). I just laugh when I see a question that is negatively voted, marked offensive a couple times AND closed. I wonder if those people closing it think they actually did something valuable with their time....

What About The Rest?

So the rest are basically questions that are running dead even at 0 votes, or have positive votes. If this is the case... doesn't that mean someone is finding value in the question? This group is pretty varied. Some of them are polls on the best X, where X can range from programming comics to the best reference guide. Some people don't like polls, but I personally find them entertaining to participate in, and even sometimes find truly useful information.

You know who else likes polls on Stack Overflow? Joel Spolsky. That's right, the cofounder of the site talks about polls in his launch post, while Jeff grits his teeth when Joel mentions them on the podcast.

A lot of the other questions are discussions, which I've already mentioned I find value in. They help build the community, and provide outlets for your ideas or disagreements with your felow programmers.

Even more of the gray zones are borderline programming questions... usually they are somehow of interest to programmers, or IT people in general, so it seems to me they have a home there (or should, anyways). These range from Virtual Machine questions, to general troubleshooting problems with an OS or application that programmers will typically use. If they are tagged right, why can't they be on the site? If you are looking in your comfort zone of tags, you will never see the VirtualBox, or Ubuntu questions, so why can't someone get real use out of the site on such marginally programming related subjects?

So What Makes Reddit Better? I Heard it Sucks!

It seems a lot of people have lost faith in Reddit, and feel it has nothing left to offer. I find it quite the contrary... it is so addictive that I have to avoid it if I want to get any work done. The key to Reddit's success, I think, is the clever combination of subgroups and voting. The voting lets the interesting topics rise to the top. The subgroups keep those topics interesting for those looking at the group. That's why I always only go to programming.reddit.com, because every time I go there, I see really interesting articles about programming. It NEVER lets me down.

Stack Overflow could do the exact same thing with their implementation of tags and voting. Filter the interesting stuff to the top with voting, and keep it interesting to me with my focused set of tags. Think of each tag as a focused niche group where the stuff that rises with voting is going to be interesting to the members of that niche group. The only stuff that group of people would want to close are all stuck in other tags that don't even appear for them.

The next time you would close a question... reconsider leaving it open for someone in that niche group that would enjoy participating in the question.

So, that's the key, in my view. Reddit trusts the cloud and refines it with subgroups and it works. Jeff Atwood doesn't trust the cloud, and he turns people like me away who favor more open systems.

What of Proof?

Well, I'm not going to compile a long list of questions to prove my point... I've seen the proof with my eyes, and if I haven't convinced you that closing questions are unnecessary, I never will. I will, however, give you a short list of some questions I reopened because I felt strongly that they shouldn't have been closed. If you agree that any of these shouldn't have been closed, then maybe there are more that shouldn't have been closed.

The truly sad thing is that some questions that shouldn't be closed lose their chance at getting a good answer because someone was denied the ability to provide a good answer because of 1 selfish person that felt the need to push his/her opinion on everyone else.
The first was interesting to me because I actually want a low energy low cost linux box. Jeff Atwood has himself blogged about this on his programming blog, so it seems appropriate.

The second and third are VM questions... I use VMs daily for my programming work, so their effective use is of much interest to me (though I have not run into these particular issues).

The final one was something I wanted to thrive because the answers make for entertaining reads. Why can't we live with some humor now and then?

Final Thoughts

So, before I relinquish my control over you (as I apparently have some control, if you've gotten this far), I want to say a couple last minute things. I weened myself off Stack Overflow because I was sick of seeing questions closed that shouldn't be. I think it is a travesty to see people force their opinion of what to discuss on others. Though the number of closed questions may be small, it is unjust enough in my opinion that Stack Overflow is not worth my traffic, or my answers. Furthermore, it makes me depressed to see Jeff have so little faith in the system and community he built. Despite drastically reducing my usage, I reserve the right to ask questions when I have one I need answered... after all, I wouldn't be this impassioned if it weren't a good site!

If you don't like closed questions either, email Jeff Atwood (you can find his email on his blog if you look for it). Alternatively, open a uservoice ticket. The more people opening tickets the better... he might submit to pressure if he has to get rid of the tickets over and over. Just stick to 1 ticket though... unique users posting requests is a lot better than abuse of the uservoice system.

UPDATE:
cky brings up a great point in the comments that removing the close question feature will also do harm in causing other users to leave... a compromise between the 2 camps is probably the best solution. Some system of check/balance for the close question feature would be good, such as a simple majority vote. Other fair alternatives would be welcome in my book as well.

Sunday, October 12, 2008

Till Death do we Rewrite

It's been a while since it was written, but I recently read an interesting article from Joel Spolsky. It's from 2000, but it is of course still very relevant.

The article was about rewriting code, something he argues you should never do. It might be more fun and appealing to play devil's advocate, but I can't for this one. Joel is dead on here.

In my last job I was dealing with GUI code primarily. There were a few of us working on this section of the code, so it was by no means all my own work. One particular component of it was written by someone else who ended up moving on to another part of the project, so me and a few others inherited the work.

There came a time to expand on this component, and so I looked over what was done. It seemed somewhat awkwardly implemented, and I felt I could do a lot better if I started from scratch. There were events being raised and passed on 3 levels deep in the object hierarchy for no apparent reason, and the object structure itself seemed a bit awkward. It also got a bit slow under certain circumstances. Thus, I plowed ahead with a rewrite.

I started out strong. I had objects that correlated logically with what was being displayed on the screen. I had a simple structure that seemed a lot clearer and cleaner to me. It was working, and it seemed to even be faster. I was proud of what I had created (at least, at the time I was).

Then time went on and the GUI got more complicated. It sprouted the hairs that Joel talks of, and started getting slow for no apparent reason (but in different ways from the original implementation). All in all, the end result started looking a lot more like what had been there than I liked to admit.

The rewrite wasn't a total failure... it worked, and some of the details had definitely been improved, but at the cost of some new hairs that I would rather go back and fix. However, in retrospect, it would have been a much better idea to just dig in with the existing code and salvage what I could, refactoring what really needed improvement. I walked away with a valuable lesson though... whenever you want to rewrite code... don't. What is already there can work if you give it a chance. Refactor what doesn't work, and make the unclear sections more clear.

Learning to resist the urge to totally rewrite code is an important lesson I needed in order to become a better programmer.

Thursday, October 9, 2008

Please Hold on the Interfaces

I just got my first Notable Question badge over on stackoverflow.com for what seemed to turn into a rather controversial question. At the time, I was just ranting based on a question I had just read where all the answers suggested the use of an interface, when it didn't really seem to need one. Even more egregious, the interface was basically being proposed just to support testing.

To spare you a little reading, the basic premise of my question was: interfaces are overused, and it sucks when they are being overused, so what is it I'm missing, if anything?

This got some people to go NUTS over it! I mean, literally these people went wildly crazy, spouting nonsense... listen to this one, which was probably one of the worst responses I saw:

An issue with favouring mocking (or any kind of runtime substition) through subclassing, rather than through an interface, is that you can only override the behaviour of virtual members.

Invoking a virtual member requires an additional level of indirection via a Vtable and is slower than a final member. Whereas using an interface adds a compile-time overhead, but using virtual members adds a runtime overhead to every member invocation.

Surely, adding an interface **just to support mocking** is far preferable to slowing down your entire app **just to suport mocking**?

My preference, therefore, is to favour 'final by default'.



Talk about premature optimization!!!! This guy wants to avoid polymorphism because of the runtime costs?!?!? I can't believe how STUPID people can get sometimes! I mean, seriously! I try not to get angry when I'm on the Internet, because flame wars and trolling gets us nowhere, but c'mon now. A v-table call is going to be... what... nanoseconds? Even faster? So small we can't even reliably measure it??? And he thinks he has a net gain by avoiding that cost? I sure wouldn't want to be the one maintaining HIS code!

OK, I'm getting off topic. Interfaces. Those things that pretend to be a class, but have no details whatsoever. I don't mind them. Sometimes they can be quite helpful. However, like all tools, we must stop and analyze why we are going to use the tool, and what will we gain by doing it. If you pound on a screw with a hammer, you might just get it in there, but did you go about it the best way?

There are times when interfaces make a lot more sense in places they don't seem to belong, such as perhaps frameworks... though this guy rightfully pointed out that they also lock you in and restrict change, especially in frameworks (where you can't control who has implemented it).

So here's my biggest beef with interfaces: using one when you have a single implementation class.

That's it. That's the single most spine-curling, eye-gouging use of interfaces that will make me want to vomit all over you if you suggest it.

If you are a lover of interfaces, this may come as shocking news (as I sadly found out). Why is it so bad? You get to increase testability, right? You make your class more extensible, right? You are programming to an interface, right?

WRONG! No, no, no, no.... NO NO NO!

Testability

First of all, testability has nothing to do with whether you have an interface or not. It has to do with whether or not the dependencies you have in your class can be mocked out and altered so that you can ensure that the class works properly regardless of what those dependencies spew at you.

For example, let's say you have an object that represents a customer, which is represented in the database. Perhaps one of the worst things you can do is load the data for that object from the database from within the constructor. At this point, you can't get around constructing your object without depending on the database. Now, if you want to write a test for a DIFFERENT class that uses this customer object, you all of a sudden have to have the database up and running. This is bad because it makes the test a lot more complicated than it needs to be.

Now, if you are in the Interface Camp, you might be thinking... hah! If you define an interface for that class, you can get around that dependency! This is true, but it's also the easy way out. Singletons are also the easy way out to a lot of problems, but you don't hear many people clamoring for singletons everywhere, do you? The easy way out is oftentimes NOT the clean or best way out. Instead of shrouding your class in an evil interface, just move the database initialization to a protected method that the constructor calls. Now you can override the protected method in subclasses or mock classes and no longer depend on the database!

Extensibility...?

I just don't understand why people think classes are less extensible than interfaces. It just isn't true, damnit!! I've been extending classes left and right when I see the need, and I have never had a problem. The trick is, again, to avoid dependencies.

If you are doing something that can't be changed in a subclass, you might be doing something wrong. The constructor is the first place to look... is there behavior in there that does something that might be unwanted in an alternate version of this class? Is there a static initializer that is doing something fundamental to the class that can't be changed in a subclass? As long as you have only virtual methods (believe me, the "performance hit" is worth it... and it's why I love Java's default virtual and hate C#'s default final), and your constructors and static initializers are clean, you can reimplement the entire class as if it had been an interface all along! It's really not that hard, and if you are active with Test Driven Development, you will find the classes often come out that way anyways.

Programming to an INTERFACE....?!?!

Someone else said it better. I don't remember who, and I don't remember where. What do I look like, google? Find it for your own damn self! Well... the gist is, "programming to an interface" does not mean literally to a programming language construct called an interface... it means programming to a contract between you and users of your class... i.e. the publicly visible methods, fields and constructors agreed upon between you and clients of your class.

Just because "interface" is a keyword in some programming languages, doesn't mean we can't speak about "interface" as an English word used to describe what we are exposing.

Appropriate Uses??

I'm not going to lay out a bunch of rules on when you should use an interface. Deciding what tool to solve a particular problem is often somewhat of an art more than an algorithm, so you really have to look at the problem and just decide for yourself. You may end up using interfaces in an inappropriate time... but hey, we all make mistakes! Refactor it when you find a better way... you got all those tests to deal with it, right?

Without going into too much detail, I think a good rule of thumb for when to use an interface effectively is to use it when the thing you are trying to build doesn't lend itself to a default implementation, or you literally HAVE to do it... such as the case when you absolutely need to extend 2 classes (pick one, and extract an interface). However, there can be ways around needing the interface in these cases, but if the interface is cleaner, go for it.

You WILL Regret it Later!

No, you won't, but some claim you will, like this guy:

Sure, sometimes you may not ever have an alternate implementation of ISomeFoo in which case it might have been a waste for that particular component. But if you ever DO need to have an alternate implementation, going and changing those 50 references to concrete PetrolSomeFoo to concrete HydrogenSomeFoo will hurt really bad, especially if someone of the changes involve other applications or integration scenarios.

So, my argument is to let the IDE do the work for you. Extracting an interface and replacing all references is not all that hard of a task, especially given the help of a good IDE. If you are developing a framework where you don't control client code... this is one of the few cases where more interfaces might make sense, unless you have the balls to break backwards compatibility. If you have the ability to only use a tool when you need it, I say save on using that tool unless it really solves your immediate problem cleaner than other solutions.

Finish Already!

So the next time you reach for the interface keyword, stop and consider to yourself... is this really the best solution? Am I really solving this problem cleanly and elegantly with interfaces, or am I just hacking around a problem without considering how else I might solve it?