apiaryio / dredd

Language-agnostic HTTP API Testing Tool
https://dredd.org
MIT License
4.18k stars 279 forks source link

Parse Error on DELETE with 204 Response and empty body #468

Closed Indemnity83 closed 7 years ago

Indemnity83 commented 8 years ago

I'm uncertain if this is related to #196

I have a couple DELETE methods defined similar to the following in my API Blueprint:

### Delete a Type [DELETE /types/{type}]

+ Parameters
    + type: 1 (number, required)
        id of type to fetch

+ Response 204

but when I run the tests everything passes except the DELETEs, each shows a similar result to this:

pass: POST /types duration: NaNms
pass: GET /types?limit=20&skip=0&order=name duration: NaNms
pass: GET /types/1?limit=20&skip=0&order=name duration: NaNms
pass: PUT /types/1 duration: NaNms
error: DELETE /types/1 duration: 82ms
error: Error: Parse Error
  at Error (native)
  at Socket.socketOnData (_http_client.js:305:20)
  at emitOne (events.js:77:13)
  at Socket.emit (events.js:169:7)
  at readableAddChunk (_stream_readable.js:146:16)
  at Socket.Readable.push (_stream_readable.js:110:10)
  at TCP.onread (net.js:523:20)

< ... additional tests run... >

info: Displaying failed tests...
fail: DELETE /types/1 duration: 82ms
fail: 
request: 
body: 

headers: 
    User-Agent: Dredd/1.0.10 (Linux 3.19.0-25-generic; x64)

uri: /types/1
method: DELETE

expected: 
headers: 

body: 

statusCode: 204

actual: 
statusCode: 204
headers: 
    server: nginx/1.8.0
    content-type: text/html; charset=UTF-8
    connection: close
    cache-control: no-cache
    date: Wed, 04 May 2016 06:20:39 GMT

body: 

This looks to me like the actual matches the expected (unless the content-type is throwing it off?) so what gives here.

honzajavorek commented 8 years ago

Thanks for reporting this! It looks like Dredd is trying to parse the empty body as JSON and fails, although there's no content type specified. It's definitely a bug.

galenandrew commented 8 years ago

@honzajavorek is this a bug with dredd or gavel? If you point me in the right direction, I'd be happy to take a stab at a PR.

honzajavorek commented 8 years ago

@galenandrew Great to hear that! I'll try to look into it and give you some pointers.

honzajavorek commented 8 years ago

I'm sorry, I didn't have time to get back to this. Looks like @netmilk could provide some input. It seems that setting the content type of the empty response to text/plain could force Dredd to treat the empty body as empty text/plain string. It could work as a workaround, but I didn't try it out yet.

mcurry7 commented 8 years ago

@honzajavorek @netmilk I have recently run into an issue with Dredd that is in line with this bug. I am sending a delete request with the status code of 204 to my server. Unfortunately as we use Luracast Restler it is over writing the content type of the request is getting over written so we are unable to change that behavior as 2/3 of our requests require this functionality. There might be a way to handle this with hooks that I am unaware of, however this seems like a blatant bug due to the fact that a 204 response wont have a body and has no need to be parsed for a proper workflow with the 204 response code. Any insight on the fix of this issue would be appreciated. Thank you.

netmilk commented 8 years ago

Hey! I proposed a solution and came up with a temporary fix for this here #556. Your thoughts and suggestions are welcome. I'd appreciate if anyone could actually try it out if it works for a real-life API. If it does I'd make it a part of Dredd.

honzajavorek commented 7 years ago

Hi folks, please check out a new proposed behavior and let me know if that would work for you: https://github.com/apiaryio/dredd/issues/556#issuecomment-252206782

jakul commented 7 years ago

Here is a temporary workaround I've written (using Python hooks)


@dredd_hooks.before_each_validation
def fix_204_responses(transaction):
    """
    Dredd has a bug which expects the response body to be valid JSON on 204
    responses, but the HTTP Spec says the body should be empty.
    https://github.com/apiaryio/dredd/issues/196
    """
    if '> 204' not in transaction['name']:
        return

    if not transaction['real']['body'] and \
            transaction['real']['statusCode'] == 204:
        # Make the body content valid JSON.
        transaction['real']['body'] = '{}'
        transaction['expected']['body'] = '{}'
honzajavorek commented 7 years ago

I've decided to close duplicates. The main issue for this problem is now https://github.com/apiaryio/dredd/issues/556. Please continue any discussions and add workarounds there. Thanks!