From 346a9f33ddf89d0e88100777c66fa580980b1b2b Mon Sep 17 00:00:00 2001 From: Ahmed Abdelhalim Date: Sun, 31 Mar 2024 03:00:49 +0200 Subject: Update the styles and add old exception blog post --- .../blog/exception-handling/images/code-review.jpg | Bin 0 -> 45533 bytes content/blog/exception-handling/index.md | 389 +++++++++++++++++++++ 2 files changed, 389 insertions(+) create mode 100644 content/blog/exception-handling/images/code-review.jpg create mode 100644 content/blog/exception-handling/index.md (limited to 'content/blog') diff --git a/content/blog/exception-handling/images/code-review.jpg b/content/blog/exception-handling/images/code-review.jpg new file mode 100644 index 0000000..a2ab678 Binary files /dev/null and b/content/blog/exception-handling/images/code-review.jpg differ diff --git a/content/blog/exception-handling/index.md b/content/blog/exception-handling/index.md new file mode 100644 index 0000000..f6aeda4 --- /dev/null +++ b/content/blog/exception-handling/index.md @@ -0,0 +1,389 @@ +--- +title: Clean code and the exception handling +date: 2024-03-30 +draft: false +--- + +Exceptions are as old as programming itself. Back in the days when programming +was done in hardware, or via low-level programming languages, +exceptions were used to alter the flow of the program, +and to avoid hardware failures. Today, Wikipedia [defines exceptions][1] as: + +> the process of responding to the occurrence of exceptions +> – anomalous or exceptional conditions requiring special processing – during the execution of a program. +> In general, an exception breaks the normal flow of execution and executes a pre-registered exception handler... + +So, exceptions require special treatment, and an unhandled exception may cause unexpected behavior. +The results are often spectacular. In 1996, the famous [Ariane 5 rocket launch failure][2] +was attributed to an unhandled overflow exception. [History’s Worst Software Bugs][3] +contains some other bugs that could be attributed to unhandled or miss-handled exceptions. + +Over time, these errors, and countless others (that were, perhaps, not as dramatic, +but still catastrophic for those involved) contributed to the impression that **exceptions are bad**. + +But exceptions are a fundamental element of modern programming; they exist to make our software better. +Rather than fearing exceptions, we should embrace them and learn how to benefit from them. +In this article, we will discuss how to manage exceptions elegantly, +and use them to write clean code that is more maintainable. + +# Exception Handling + +With the rise of [object-oriented programming (OOP)][4], +exception support has become a crucial element of modern programming languages. +A robust exception handling system is built into most languages, +nowadays. For example, Ruby provides for the following typical pattern: + +```rb +begin + do_something_that_might_not_work! +rescue SpecificError => e + do_some_specific_error_clean_up + retry if some_condition_met? +ensure + this_will_always_be_executed +end +``` +There is nothing wrong with the previous code. But overusing these patterns will cause code smells, +and won’t necessarily be beneficial. Likewise, misusing them can actually do a lot of harm to your code base, +making it brittle, or obfuscating the cause of errors. + +The stigma surrounding exceptions often makes programmers feel at a loss. +It’s a fact that exceptions can’t be avoided, but we are often taught they must be dealt with swiftly and decisively. +As we will see, this is not necessarily true. Rather, we should learn the art of handling exceptions gracefully, +making them harmonious with the rest of our code. + +Following are some recommended practices that will help you embrace exceptions +and make use of them and their abilities to keep your code **maintainable**, **extensible**, and **readable**: + +- **maintainability**: Allows us to easily find and fix new bugs, + without the fear of breaking current functionality, introducing further bugs, + or having to abandon the code altogether due to increased complexity over time. + +- **extensibility**: Allows us to easily add to our code base, + implementing new or changed requirements without breaking existing functionality. + Extensibility provides flexibility, and enables a high level of reusability for our code base. + +- **readability**: Allows us to easily read the code and discover it’s purpose without spending too much time digging. + This is critical for efficiently discovering bugs and untested code. + +These elements are the main factors of what we might call cleanliness or quality, +which is not a direct measure itself, but instead is the combined effect of the previous points, +as demonstrated in this comic: + +Code Review Comic https://www.osnews.com/story/19266/wtfsm/ +Source: [https://www.osnews.com/story/19266/wtfsm/][5] + +With that said, let’s dive into these practices and see how each of them affects those three measures. + +*Note: post examples are from Ruby, and the Rails framework but all of the constructs demonstrated here +have equivalents in the most common OOP languages/frameworks.* + +## 1. Always create your own `ApplicationError` hierarchy + +Most languages come with a variety of exception classes, +organized in an inheritance hierarchy, like any other OOP class. +To preserve the readability, maintainability, and extensibility of our code, +it’s a good idea to create our own sub-tree of application-specific exceptions +that extend the base exception class. +Investing some time in logically structuring this hierarchy can be extremely beneficial. For example: + +```rb +class ApplicationError < StandardError; end + +# Validation Errors +class ValidationError < ApplicationError; end +class RequiredFieldError < ValidationError; end +class UniqueFieldError < ValidationError; end + +# HTTP 4XX Response Errors +class ResponseError < ApplicationError; end +class BadRequestError < ResponseError; end +class UnauthorizedError < ResponseError; end +# ... +``` +Having an extensible, comprehensive exceptions package for our application makes handling +these application-specific situations much easier. +For example, we can decide which exceptions to handle in a more natural way. +This not only boosts the readability of our code, +but also increases the maintainability of our applications and libraries. + +From the **readability** perspective, it’s much easier to follow the logic of: +```rb +rescue ValidationError => e +``` +Than to read/follow the logic of: +```rb +rescue RequiredFieldError, UniqueFieldError, ... => e +``` +From the **maintainability** perspective, say, for example, +we are implementing a JSON API, and we have defined our own `ClientError` with several subtypes, +to be used when a client sends a bad request. If any one of these is raised, +the application should render the JSON representation of the error in its response. +It will be easier to fix, or add logic, to a single block that handles `ClientErrors` +rather than looping over each possible client error and implementing the same handler code for each. + +In terms of **extensibility**, if we later have to implement another type of client error, +we can trust it will already be handled properly here. + +Moreover, this does not prevent us from implementing additional special handling for +specific client errors earlier in the call stack, or altering the same exception object along the way: + +```rb +# app/controller/pseudo_controller.rb +def authenticate_user! + fail AuthenticationError if token_invalid? || token_expired? + User.find_by(authentication_token: token) +rescue AuthenticationError => e + report_suspicious_activity if token_invalid? + raise e +end + +def show + authenticate_user! + show_private_stuff!(params[:id]) +rescue ClientError => e + render_error(e) +end +``` +As you can see, raising this specific exception didn’t prevent us from being able to handle it on different levels, +altering it, re-raising it, and allowing the parent class handler to resolve it. + +**Two things to note here:** +- Not all languages support raising exceptions from within an exception handler. +- In most languages, raising a new exception from within a handler will cause the original exception to be lost forever, + so it’s better to re-raise the same exception object (as in the above example) + to avoid losing track of the original cause of the error. (Unless you are doing this [intentionally][6]). + +## 2. Never `rescue Exception` + +That is, never try to implement a catch-all handler for the base exception type. +Rescuing or catching all exceptions wholesale is never a good idea in any language, +whether it’s globally on a base application level, or in a small buried method used only once. +We don’t want to `rescue Exception` because it will obfuscate whatever really happened, +**damaging both maintainability and extensibility.** +We can waste a huge amount of time debugging what the actual problem is, for example: + +```rb +# main.rb +def bad_example + do_some_analysis! +rescue Exception + report_analysis_error +end +``` +```rb +# elsewhere.rb +def do_some_analysis! + # yada yada + retrun summary +end +``` + +You might have noticed the error in the previous example; +`return` is misspelled. Although modern editors provide some protection against this specific type of syntax error, +this example illustrates how `rescue Exception` does harm to our code. +At no point is the actual type of the exception (in this case a `NoMethodError`) addressed, or intended to be addressed. + +## 3. Never `rescue` more exceptions than you need to + +The point 2 is a specific case of this general rule: +We should always be careful not to over-generalize our exception handlers. +The reasons are the same; whenever we catch more exceptions than we should, +we end up hiding parts of the application logic from higher levels of the application, +not to mention suppressing the developer’s ability to handle the exception themselves. +**This severely affects the extensibility and maintainability of the code.** + +If we do attempt to handle different exception subtypes in the same handler, +we introduce fat code blocks that have too many responsibilities. +For example, if we are building a library that consumes a remote API, handling a `MethodNotAllowedError` (HTTP 405), +is usually different from handling an `UnauthorizedError` (HTTP 401), even though they are both `ResponseError`s. + +As we will see, often there exists a different part of the application +that would be better suited to handle specific exceptions in a more [DRY][7] way. + +So, define the [single responsibility][8] of your class or method, +and **handle the bare minimum of exceptions that satisfy this responsibility requirement**. +For example, if a method is responsible for getting stock info from a remote a API, +then it should handle exceptions that arise from getting that info only, +and leave the handling of the other errors to a different method designed specifically for these responsibilities: + +```rb +def get_info + begin + response = HTTP.get(STOCKS_URL + "#{@symbol}/info") + + fail AuthenticationError if response.code == 401 + fail StockNotFoundError, @symbol if response.code == 404 + return JSON.parse response.body + rescue JSON::ParserError + retry + end +end +``` +Here we defined the contract for this method to only get us the info about the stock. +It **handles endpoint-specific errors**, such as an incomplete or malformed JSON response. +It doesn’t handle the case when authentication fails or expires, or if the stock doesn’t exist. +These are someone else’s responsibility, and are explicitly passed up the call stack +where there should be a better place to handle these errors in a DRY way. + +## 4. Resist the urge to handle exceptions immediately + +This is the complement to the point 3. +An exception can be handled at any point in the call stack, and any point in the class hierarchy, +so knowing exactly where to handle it can be mystifying. +To solve this issue, many developers opt-in to handle any exception as soon as it arises, +but investing time in thinking this through will usually result +in finding a more appropriate place to handle specific exceptions. + + +One common pattern that we see in [Rails][9] [applications][10] (even in the [getting started tutorial][11]) +is the following controller method: + +```rb +# app/controllers/client_controller.rb + +def create + @client = Client.new(params[:client]) + if @client.save + render json: @client + else + render json: @client.errors + end +end +``` +(Note that although this is not technically an exception handler, functionally, it serves the same purpose, +[since `@client.save` only returns `false` when it encounters an exception][12].) + +In this case, however, repeating the same error handler in every controller action is the opposite of DRY, +and damages **maintainability** and **extensibility**. +Instead, we can make use of the special nature of exception propagation, +and handle them only once, in the [parent controller class][13], `ApplicationController`: + +```rb +# app/controllers/client_controller.rb + +def create + @client = Client.create!(params[:client]) + render json: @client +end +``` +```rb +# app/controller/application_controller.rb + +rescue_from ActiveRecord::RecordInvalid, with: :render_unprocessable_entity + +def render_unprocessable_entity(e) + render \ + json: { errors: e.record.errors }, + status: 422 +end +``` + +This way, we can ensure that all of the `ActiveRecord::RecordInvalid` errors +are properly and DRY-ly handled in one place, on the base `ApplicationController` level. +This gives us the freedom to fiddle with them if we want to handle specific cases at the lower level, +or simply let them propagate gracefully. + +## 5. Not all exceptions need handling + +When developing a gem or a library, many developers will try to encapsulate the functionality +and not allow any exception to propagate out of the library. +But sometimes, it’s not obvious how to handle an exception until the specific application is implemented. + +Let’s take [`ActiveRecord`][14] as an example of the ideal solution. +The library provides developers with two approaches for completeness. +The [`save`][15] method handles exceptions without propagating them, simply returning `false`, +while [`save!`][16] raises an exception when it fails. +This gives developers the option of handling specific error cases differently, +or simply handling any failure in a general way. + +But what if you don’t have the time or resources to provide such a complete implementation? +In that case, if there is any uncertainty, it is best to **expose the exception**. + +Here’s why: We are working with changing requirements almost all the time, +and making the decision that an exception will always be handled in a specific +way might actually harm our implementation, damaging **extensibility** and **maintainability**, +and potentially adding huge technical debt, especially when developing libraries. + +Take the earlier example of a stock API consumer fetching stock prices (point 3.). +We chose to handle the incomplete and malformed response on the spot, +and we chose to retry the same request again until we got a valid response. +But later, the requirements might change, such that we must fall back to saved historical stock data, +instead of retrying the request. + +At this point, we will be forced to change the library itself, +updating how this exception is handled, because the dependent projects won’t handle this exception. +(How could they? It was never exposed to them before.) +We will also have to inform the owners of projects that rely on our library. +This might become a nightmare if there are many such projects, +since they are likely to have been built on the assumption that this error will be handled in a specific way. + +Now, we can see, **how this situation degrades the library’s usefulness, extensibility, and flexibility.** + +So here is the bottom line: **if it is unclear how an exception should be handled, let it propagate gracefully.** +There are many cases where a clear place exists to handle the exception internally, +but there are many other cases where exposing the exception is better. +So before you opt into handling the exception, just give it a second thought. +**A good rule of thumb is to only insist on handling exceptions when you are interacting directly with the end-user.** + +## 6. Follow the convention + +The implementation of Ruby, and, even more so, Rails, follows some naming conventions, +such as distinguishing between `method_names` and `method_names!` with a “bang.” +In Ruby, the bang indicates that the method will alter the object that invoked it, +and in Rails, it means that the method will raise an exception if it fails to execute. +**Try to respect the same convention, especially if you are going to open-source your library.** + +If we were to write a new `method!` with a bang in a Rails application, we must take these conventions into account. +There is nothing forcing us to raise an exception when this method fails. + +Another Ruby convention, attributed to [Jim Weirich][17] ([Thanks for everything Jim. We will miss you][18]), +is to use `fail` to indicate method failure, and only to use `raise` if you are re-raising the exception. + +> An aside, because I use exceptions to indicate failures, +> I almost always use the `fail` keyword rather than the `raise` keyword in Ruby. +> Fail and raise are synonyms so there is no difference except that `fail` +> more clearly communicates that the method has failed. +> The only time I use `raise` is when I am catching an exception and re-raising it, +> because here I’m not failing, but explicitly and purposefully raising an exception. +> This is a stylistic issue I follow, but I doubt many other people do. + +[Source][19] + +Many other language communities have adopted conventions like these around how exceptions are treated, +and **ignoring these conventions will damage the readability and maintainability of our code.** + +## 7. Logger.log(everything) + +This practice doesn’t solely apply to exceptions, +but if there’s one thing that should always be logged, it’s an exception. + +*Special thanks to [Avdi Grimm][20] and his awesome talk [Exceptional Ruby][21], +which helped a lot in the making of this article.* +*This article was originally published with the help of an editor on [Toptal in September 2017][22].* + + +[1]: https://en.wikipedia.org/wiki/Exception_handling +[2]: https://en.wikipedia.org/wiki/Ariane_flight_V88#Launch_failure +[3]: https://www.wired.com/2005/11/historys-worst-software-bugs/ +[4]: https://en.wikipedia.org/wiki/Object-oriented_programming +[5]: https://www.osnews.com/story/19266/wtfsm/ +[6]: https://github.com/rails/rails/blob/52bcbde60d101536d531337b9edb83a94604bc74/actionpack/lib/action_dispatch/http/request.rb#L359-L361 +[7]: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself +[8]: https://en.wikipedia.org/wiki/Single_responsibility_principle +[9]: https://gitlab.com/gitlab-org/gitlab/-/blob/000a47ea85408887635bf952a123f76a99ea3144/app/controllers/projects/error_tracking/projects_controller.rb#L21-L28 +[10]: https://github.com/mastodon/mastodon/blob/90eb4a5d01f1f1e8a76e7dc814eae992a9c1ee16/app/controllers/api/base_controller.rb#L79-L90 +[11]: https://guides.rubyonrails.org/getting_started.html +[12]: https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/persistence.rb#L120 +[13]: https://github.com/mastodon/mastodon/blob/90eb4a5d01f1f1e8a76e7dc814eae992a9c1ee16/app/controllers/application_controller.rb#L28-L36 +[14]: https://github.com/rails/rails/tree/4-2-stable/activerecord/lib/active_record +[15]: https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/persistence.rb#L119-L123 +[16]: https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/persistence.rb#L141-L143 +[17]: https://en.wikipedia.org/wiki/Jim_Weirich +[18]: https://github.com/jimweirich/wyriki/commit/d28fac7f18aeacb00d8ad3460a0a5a901617c2d4 +[19]: https://avdi.codes/jim-weirich-on-exceptions/ +[20]: https://avdi.codes/about/ +[21]: https://www.youtube.com/watch?v=qEHn46YtRM8 +[22]: https://www.toptal.com/abap/clean-code-and-the-art-of-exception-handling -- cgit v1.2.3