change error interface: read user message from locales

Pedro Belo · Pedro Belo · commit 22228510dc58 · 2016-01-06T20:14:24.000-08:00
diff --git a/lib/pliny/errors.rb b/lib/pliny/errors.rb
@@ -1,86 +1,61 @@
+require "i18n"
+
 module Pliny
   module Errors
     class Error < StandardError
-      attr_accessor :id, :metadata
 
-      def self.render(error)
-        headers = { "Content-Type" => "application/json; charset=utf-8" }
-        data = { id: error.id, message: error.message }.merge(error.metadata)
-        [error.status, headers, [MultiJson.encode(data)]]
+      class << self
+        attr_accessor :error_class_id, :error_class_status
+
+        def render(error)
+          headers = { "Content-Type" => "application/json; charset=utf-8" }
+          data = { id: error.id, message: error.user_message }.merge(error.metadata)
+          [error.status, headers, [MultiJson.encode(data)]]
+        end
       end
 
-      def initialize(message, id: nil, metadata: {})
-        @id = id
+      attr_accessor :id, :status, :metadata, :user_message
+
+      def initialize(id=nil, metadata: {})
+        @id = (id || self.class.error_class_id).to_sym
+        @status = self.class.error_class_status
         @metadata = metadata
-        super(message)
+        @user_message = I18n.t("errors.#{@id}")
+        super(@id.to_s)
       end
     end
 
-    class HTTPStatusError < Error
-      attr_accessor :status
-
-      def initialize(message=nil, options={})
-        meta = Pliny::Errors::META[self.class]
-        message ||= "#{meta[1]}."
-        options[:id] ||= meta[1].downcase.tr(' ', '_').to_sym
-        @status = options.delete(:status) || meta[0]
-        super(message, options)
+    def self.MakeError(status, id)
+      Class.new(Pliny::Errors::Error) do
+        @error_class_id = id
+        @error_class_status = status
       end
     end
 
-    class BadRequest < HTTPStatusError; end                   # 400
-    class Unauthorized < HTTPStatusError; end                 # 401
-    class PaymentRequired < HTTPStatusError; end              # 402
-    class Forbidden < HTTPStatusError; end                    # 403
-    class NotFound < HTTPStatusError; end                     # 404
-    class MethodNotAllowed < HTTPStatusError; end             # 405
-    class NotAcceptable < HTTPStatusError; end                # 406
-    class ProxyAuthenticationRequired < HTTPStatusError; end  # 407
-    class RequestTimeout < HTTPStatusError; end               # 408
-    class Conflict < HTTPStatusError; end                     # 409
-    class Gone < HTTPStatusError; end                         # 410
-    class LengthRequired < HTTPStatusError; end               # 411
-    class PreconditionFailed < HTTPStatusError; end           # 412
-    class RequestEntityTooLarge < HTTPStatusError; end        # 413
-    class RequestURITooLong < HTTPStatusError; end            # 414
-    class UnsupportedMediaType < HTTPStatusError; end         # 415
-    class RequestedRangeNotSatisfiable < HTTPStatusError; end # 416
-    class ExpectationFailed < HTTPStatusError; end            # 417
-    class UnprocessableEntity < HTTPStatusError; end          # 422
-    class TooManyRequests < HTTPStatusError; end              # 429
-    class InternalServerError < HTTPStatusError; end          # 500
-    class NotImplemented < HTTPStatusError; end               # 501
-    class BadGateway < HTTPStatusError; end                   # 502
-    class ServiceUnavailable < HTTPStatusError; end           # 503
-    class GatewayTimeout < HTTPStatusError; end               # 504
-
-    # Messages for nicer exceptions, from rfc2616
-    META = {
-      BadRequest                   => [400, 'Bad request'],
-      Unauthorized                 => [401, 'Unauthorized'],
-      PaymentRequired              => [402, 'Payment required'],
-      Forbidden                    => [403, 'Forbidden'],
-      NotFound                     => [404, 'Not found'],
-      MethodNotAllowed             => [405, 'Method not allowed'],
-      NotAcceptable                => [406, 'Not acceptable'],
-      ProxyAuthenticationRequired  => [407, 'Proxy authentication required'],
-      RequestTimeout               => [408, 'Request timeout'],
-      Conflict                     => [409, 'Conflict'],
-      Gone                         => [410, 'Gone'],
-      LengthRequired               => [411, 'Length required'],
-      PreconditionFailed           => [412, 'Precondition failed'],
-      RequestEntityTooLarge        => [413, 'Request entity too large'],
-      RequestURITooLong            => [414, 'Request-URI too long'],
-      UnsupportedMediaType         => [415, 'Unsupported media type'],
-      RequestedRangeNotSatisfiable => [416, 'Requested range not satisfiable'],
-      ExpectationFailed            => [417, 'Expectation failed'],
-      UnprocessableEntity          => [422, 'Unprocessable entity'],
-      TooManyRequests              => [429, 'Too many requests'],
-      InternalServerError          => [500, 'Internal server error'],
-      NotImplemented               => [501, 'Not implemented'],
-      BadGateway                   => [502, 'Bad gateway'],
-      ServiceUnavailable           => [503, 'Service unavailable'],
-      GatewayTimeout               => [504, 'Gateway timeout'],
-    }.freeze
+    BadRequest                   = MakeError(400, :bad_request)
+    Unauthorized                 = MakeError(401, :unauthorized)
+    PaymentRequired              = MakeError(402, :payment_required)
+    Forbidden                    = MakeError(403, :forbidden)
+    NotFound                     = MakeError(404, :not_found)
+    MethodNotAllowed             = MakeError(405, :method_not_allowed)
+    NotAcceptable                = MakeError(406, :not_acceptable)
+    ProxyAuthenticationRequired  = MakeError(407, :proxy_authentication_required)
+    RequestTimeout               = MakeError(408, :request_timeout)
+    Conflict                     = MakeError(409, :conflict)
+    Gone                         = MakeError(410, :gone)
+    LengthRequired               = MakeError(411, :length_required)
+    PreconditionFailed           = MakeError(412, :precondition_failed)
+    RequestEntityTooLarge        = MakeError(413, :request_entity_too_large)
+    RequestURITooLong            = MakeError(414, :request_uri_too_long)
+    UnsupportedMediaType         = MakeError(415, :unsupported_media_type)
+    RequestedRangeNotSatisfiable = MakeError(416, :requested_range_not_satisfiable)
+    ExpectationFailed            = MakeError(417, :expectation_failed)
+    UnprocessableEntity          = MakeError(422, :unprocessable_entity)
+    TooManyRequests              = MakeError(429, :too_many_requests)
+    InternalServerError          = MakeError(500, :internal_server_error)
+    NotImplemented               = MakeError(501, :not_implemented)
+    BadGateway                   = MakeError(502, :bad_gateway)
+    ServiceUnavailable           = MakeError(503, :service_unavailable)
+    GatewayTimeout               = MakeError(504, :gateway_timeout)
   end
 end
diff --git a/lib/template/config/locales/en.yml b/lib/template/config/locales/en.yml
@@ -1 +1,27 @@
 en:
+  errors:
+    bad_gateway: Bad gateway
+    bad_request: Bad request
+    conflict: Conflict
+    expectation_failed: Expectation failed
+    forbidden: Forbidden
+    gateway_timeout: Gateway timed out
+    gone: Gone
+    internal_server_error: Internal server error
+    length_required: Length required
+    method_not_allowed: Method not allowed
+    not_acceptable: Not acceptable
+    not_found: Not found
+    not_implemented: Not implemented
+    payment_required: Payment required
+    proxy_authentication_required: Proxy authentication required
+    precondition_failed: Precondition failed
+    request_entity_too_large: Request entity too large
+    request_timeout: Request timed out
+    request_uri_too_long: Requrest URI too long
+    requested_range_not_satisfiable: Requested range not satisfiable
+    service_unavailable: Service unavailable
+    too_many_requests: Too many requests
+    unauthorized: Unauthorized
+    unprocessable_entity: Unprocessable entity
+    unsupported_media_type: Unsupported media type
diff --git a/pliny.gemspec b/pliny.gemspec
@@ -16,6 +16,7 @@ Gem::Specification.new do |gem|
   gem.files = %x{ git ls-files }.split("\n").select { |d| d =~ %r{^(License|README|bin/|data/|ext/|lib/|spec/|test/)} }
 
   gem.add_dependency "activesupport",  "~> 4.1",  ">= 4.1.0"
+  gem.add_dependency "i18n",           "~> 0.7",  ">= 0.7"
   gem.add_dependency "multi_json",     "~> 1.9",  ">= 1.9.3"
   gem.add_dependency "prmd",           "~> 0.7.0"
   gem.add_dependency "sinatra",        "~> 1.4",  ">= 1.4.5"
diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb
@@ -1,36 +1,32 @@
 require "spec_helper"
 
 describe Pliny::Errors do
-  describe Pliny::Errors::Error do
-    it "takes a message" do
-      e = Pliny::Errors::Error.new("Fail.")
-      assert_equal "Fail.", e.message
-    end
-    it "takes an identifier" do
-      e = Pliny::Errors::Error.new(nil, id: :fail)
-      assert_equal :fail, e.id
-    end
+  it "bundles classes to represent the different status codes" do
+    error = Pliny::Errors::BadRequest.new
+    assert_equal :bad_request, error.id
+    assert_equal 400, error.status
+    assert_equal "Bad request", error.user_message
 
-    it "takes metadata" do
-      meta = { resource: "artists" }
-      e = Pliny::Errors::Error.new(nil, metadata: meta)
-      assert_equal meta, e.metadata
-    end
+    error = Pliny::Errors::InternalServerError.new
+    assert_equal :internal_server_error, error.id
+    assert_equal 500, error.status
+    assert_equal "Internal server error", error.user_message
   end
 
-  describe Pliny::Errors::HTTPStatusError do
-    it "includes an HTTP error that will take generic parameters" do
-      e = Pliny::Errors::HTTPStatusError.new("error", id: :foo, status: 499)
-      assert_equal :foo, e.id
-      assert_equal 499, e.status
-      assert_equal "error", e.message
-    end
+  it "keeps the error id stored as the internal message" do
+    error = Pliny::Errors::BadRequest.new
+    assert_equal "bad_request", error.message
+  end
+
+  it "takes a custom id" do
+    error = Pliny::Errors::BadRequest.new(:invalid_json)
+    assert_equal :invalid_json, error.id
+    assert_equal 400, error.status
+  end
 
-    it "includes pre-defined HTTP error templates" do
-      e = Pliny::Errors::NotFound.new
-      assert_equal "Not found.", e.message
-      assert_equal :not_found, e.id
-      assert_equal 404, e.status
-    end
+  it "takes optional metadata" do
+    metadata = { foo: "bar" }
+    error = Pliny::Errors::BadRequest.new(:invalid_json, metadata: metadata)
+    assert_equal metadata, error.metadata
   end
 end
diff --git a/spec/middleware/rescue_errors_spec.rb b/spec/middleware/rescue_errors_spec.rb
@@ -23,7 +23,7 @@ def app
     assert_equal 503, last_response.status
     error_json = MultiJson.decode(last_response.body)
     assert_equal "service_unavailable", error_json["id"]
-    assert_equal "Service unavailable.", error_json["message"]
+    assert_equal "Service unavailable", error_json["message"]
   end
 
   it "intercepts exceptions and renders" do
@@ -32,7 +32,7 @@ def app
     assert_equal 500, last_response.status
     error_json = MultiJson.decode(last_response.body)
     assert_equal "internal_server_error", error_json["id"]
-    assert_equal "Internal server error.", error_json["message"]
+    assert_equal "Internal server error", error_json["message"]
   end
 
   it "raises given the raise option" do
