mattetti / Weasel-Diesel

DSL to describe, document and test web services
MIT License
438 stars 21 forks source link

How to specify error output? #12

Open drnic opened 12 years ago

drnic commented 12 years ago

Here is the specification of output if everything goes swimmingly:

  service.response do |response|
    response.object do |obj|
      obj.object :user do |user|
        user.string :email, :doc => "Email address"
        user.string :awsm_token, :doc => "Engine Yard Cloud API token"
      end
    end
  end

What if the service cannot process the user data and needs to halt. How can we specify the schema of error output?

mattetti commented 12 years ago

That's a good question, are you talking about for instance a special error code or a specific json structure?

My experience with web services is that we have a default way to report the error and that doesn't need to be documented in each service.

I'm open to extend the API to support, for instance @aeden was asking about explicitly specify the status code (201 instead of 200 for instance). I usually do that in the overall description but if we find a nice way to do that in the API, why not.

pheino commented 11 years ago

I'm finding the need to specify the http status code (200, 201, 202, 422, 400 etc) in order to get proper documentation and more importantly leaving it up to the API implementation to decide.

I like the idea of defining the expected response codes for success and failure. That would probably help with automated testing too. You could make sure the API properly returns http 202 Accepted. I think if your writing APIs that usually you want to respond with HTTP 400/404/422 in an error case.

One idea is to define http status codes for success and failure:

describe_service "helloworld.json" do |service|
  service.formats   :json
  service.http_verb :get
  service.success 201
  service.error 422
  # OR
  service.response.success 201
  service.response.error 422
end

The error response structure I would agree and rather leave at a higher level. I think it still make sense to define it as part of the DSL. Maybe a top level configuration block?

WeaselDiesel.error do |error|
  error.status 422
  error.response do |resp|
    resp.object do |e|
      e.array |item|
        item.attribute :message => :string
      end
    end
  end
end

# or API level
service.error do |error|
  error.status 422
  error.response do |resp|
  end
end

service.response do |response|
  response.status 201
  response.object :object do |obj|
  end
end

service.implementation do
  if @obj,save
    [201, {}, @object.to_json]
  else
    [422, {}, @object.errors.full_messages.to_json]
  end
end
kamui commented 11 years ago

@pheino: I don't think you can list errors by the status code, because each user might handle these differently. For form validation errors, some people prefer to return a 400, some return 422, some return 403, so merely describing a 422 in the DSL doesn't explain what the code is being used for.

For default errors, such as form validation errors, malformed request errors, etc. I'd suggest something like this:

WeaselDiesel.describe_error :malformed_params do |error|
  error.status 400

  error.documentation do |doc|
    doc.overall "Sending invalid JSON will result in a 400 Bad Request response."
    doc.example "Some example of malformed input"
  end

  error.response do |response|
    response.object :error do |e|
      e.string :name => :message
    end
  end
end

WeaselDiesel.describe_error :input_validation_error do |error|
  error.status 422

  error.documentation do |doc|
    doc.overall "Sending invalid fields will result in a 422 Unprocessable Entity response."
    doc.example "Some example of invalid input values"
  end

  error.response do |response|
    response.object :error do |e|
      e.string :name => :message
      e.array :errors do |node|
        node.string :name
        node.string :message
      end
    end
  end
end

They could be stored internally as a hash since they are named. The purpose of the name is so that generators like wd_sinatra could reference them when they need to know what to do in case of these errors.

Currently in wd_sinatra, say you had invalid input on a POST request, well right now wd_sinatra returns a 400 with an error message. You can define a hook method that could run in it's place, but if you're going to define it anyway in the WD DSL, you might as well have wd_sinatra describe default errors in this DSL, and then wd_sinatra could call those implementations by the error name. Then the user could override those definitions with their own definitions if they don't like the defaults.

I'm not sure about per service errors, I'll have to think about that, but @raul came up with this proposal.