mulesoft / osprey

Generate Node.JS API middleware from a RAML definition
Other
431 stars 66 forks source link

Problems parsing RAML file and/or http request via express #85

Closed pickworth closed 8 years ago

pickworth commented 8 years ago

For some reason, osprey is giving me an error "No body sent with request", when there is not supposed to be a body sent with this request. (by design) When I send one (for test purposes) it is then saying it is not specified in the RAML file. e.g.:

1)

  curl 'http://localhost:4447/api/export/pdf/hosts'

Gives:

UnsupportedMediaTypeError: No body sent with request for GET /api/export/pdf/hosts with content-type "undefined"
   at ospreyContentType

2)

  curl 'http://localhost:4447/api/export/pdf/hosts' -H "Content-Type: text/plain" -d ''

Gives:

Error: Path /export/pdf/hosts was not found in the RAML file for this application
   at notFoundHandler

3) So, when I remove the 'body' spec from the RAML, (which i have added simply for testing, see below), I get this error:

  curl 'http://localhost:4447/api/export/pdf/hosts'

This request body was discarded by Osprey. Use "/" or populate "body" in your RAML document to accept data with "/api/export/pdf/hosts"

Similarly, sending an actual body with body spec removed, as in eg 2) curl cmd above, getting same result as 2)

Kind of stuck here. Seems like a bug to me.

I am using osprey-router, with the route defined like this:

router.get('/export/{fileType}/{pageId}', {
  fileType: {
    type: 'string'
  },
  pageId: {
    type: 'string'
  }
}, export_data_handler);

(export_data_handler is a function accepting the res & req params and providing response)

The RAML spec for the endpoint is defined like this:

/export:
  /{fileType}:
    /{pageId}:
      type: data-export
      get:
        securedBy: [1]
        description: Gets a data export of `fileType = {fileType}` where `pageId = {pageId}`
        body:
          application/json:
            example: |
              {}   
          text/html:
            example: |
              {}   
          text/plain:
            example: |
              {}   
    responses:
      200:
        body:
          application/pdf:
            example: 
              !include examples/report.pdf
          application/csv:
            example: 
              !include examples/report.csv
          application/json:
            example: !include examples/export-json-data-response.json

I'm using the latest express (4.x) and npm version of osprey: $ cat package.json | grep osprey "osprey": "^0.2.0-beta.9", "osprey-mock-service": "0.0.13", "osprey-router": "^0.1.1",

Just to add also, I have several other mock API endpoints which are working fine.. Cannot figure out why this particular config does not.. I have no idea what the error message means by ( Use "/" ) either..

blakeembrey commented 8 years ago

Thanks for the report, I'm taking a look right now :+1:

pickworth commented 8 years ago

Thanks :)

Let me know if you need more info, or for me to do some other tests

blakeembrey commented 8 years ago

@nmors Your body, responses, etc are indented on level with the resource, that looks like it might be your issue? Do you have information on the type: data-export? In case the method is being described in there, so I can replicate that.

blakeembrey commented 8 years ago

@nmors If possible, could I get a snippet of the RAML to replicate just the first point. I assumed the snippet there involves multiple iterations of what you were trying to do too.

pickworth commented 8 years ago

@blakeembrey the body, response, etc, indentation was just an error from copy/paste into github. it's correct in my file. I'll update it now.

resourceTypes/data-export.raml :

usage: This resourceType should be used for any export of data
description: The data export collection of <<resourcePathName>>
get:
  description: Get data <<resourcePathName>>, optionally filtered
  queryParameters:
    from:
      description: "Date range: From"
      required: false
      type: string
      example: "22-10-2014"
    to:
      description: "Date range: To"
      required: false
      type: string
      example: "22-10-2014"
    view:
      description: "The compliance standard to use"
      required: false
      type: string
      example: "iso27002"
    expanded:
      description: "Whether or not the view is expanded. Defaults to False."
      required: false
      type: boolean
      example: "true"
    detailed:
      description: "Whether or not the view is detailed. Defaults to False."
      required: false
      type: boolean
      example: "true"
  responses:
    404:
      body:
        application/json:
          example: !include ../examples/404-not-found-response.json
    204:
      body:
        application/json:
          example: !include ../examples/204-no-content-response.json
    400:
      body:
        application/json:
          example: !include ../examples/400-bad-request-response.json
    403:
      body:
        application/json:
          example: !include ../examples/403-forbidden-response.json
pickworth commented 8 years ago

api.raml (with irrelevant endpoints removed):

#%RAML 0.8
---
title: ED
protocols: [ HTTPS ]
baseUri: https://myserver.com.au/{api_root_endpoint}
version: 2.5
baseUriParameters:
  api_root_endpoint:
    description: The configured API endpoint in application config
#mediaType: application/json
securedBy: [1]

resourceTypes:
  - collection-item: !include resourceTypes/collection-item.raml
  - collection: !include resourceTypes/collection.raml
  - configuration: !include resourceTypes/configuration.raml
  - data-export: !include resourceTypes/data-export.raml
  - meta-data: !include resourceTypes/meta-data.raml
  - run-job: !include resourceTypes/run-job.raml

schemas:
 - rule: !include schemas/rules-schema.json

documentation:
 - title: API Usage
   content: !include markdown/api_usage.md

/export:
  /{fileType}:
    #uriParameters:
    #  fileType:
    #    displayName: File type
    #    type: string
    /{pageId}:
      type: data-export
      #uriParameters:
      #  pageId:
      #    displayName: The pageId
      #    type: string 
      get:
        securedBy: [1]
        description: Gets a data export of `fileType = {fileType}` where `pageId = {pageId}`
#        body:
#          application/json:
#            example: |
#              {}   
#          text/html:
#            example: |
#              {}   
#          text/plain:
#            example: |
#              {}   
        responses:
          200:
            body:
              application/pdf:
                example: !include examples/report.pdf
              application/csv:
                example: !include examples/report.csv
              application/json:
                example: !include examples/export-json-data-response.json
pickworth commented 8 years ago

The different iterations I have tried that i described, are above (commented out) you may uncomment to replicate the tests I did.

For completeness, here is how I start the API:

var api_spec = "raml/api.raml"

osprey.loadFile(api_spec)
.then(function (middleware) {
  app.disable('x-powered-by');

  // Parse Request Bodies
  //app.use(require('body-parser').json({ extended: true }));
  app.use(require('body-parser').urlencoded({ extended: true }));

  // Parsing Forms
  // var formidable = require('formidable');

  // Serve API
  app.use('/api', middleware, router);

  // Serve static
  app.use('/', express.static('static'));

  // Handle Errors
  app.use(function (err, req, res, next) {
    if(err){
        res.json({
          "error" : err
        });
    }else{
      next();
    }
  });

  // Handle Errors
  app.use(function (req, res) {
    res.status(404);
    res.json({"error" : "404"});
  });

  // Handle Errors
  app.use(function (req, res) {
    res.status(500);
    res.json({"error" : "500"});
  });
  app.listen(osprey_listen_port);
})
blakeembrey commented 8 years ago

I can't seem to replicate the 1st case, was there a body defined at that point? As for 2, looks like a bug in the error messaging (should definitely be an error since you're sending a body when it doesn't expect one, but that's a terribly wrong message). With 3, that's not actually an error, but a warning in your terminal when accessing the .body property (it's meant to be helpful, I can try making it more helpful).

Please let me know if what I said above sounds correct.

pickworth commented 8 years ago

yes, in the 1st case, the body was defined in RAML (but not in the curl request).

This sounds right. But with # 3), the request times out eventually. Why would it be trying to access the .body property, when the request is not supposed to contain a body, nor is one specified in RAML?

I also removed the body-parser middleware for testing, and getting the same results. I'm a bit lost here...

Sorry if I'm missing something obvious, this is my first time with RAML:

The error states: This request body was discarded by Osprey. Use "/" or populate "body" in your RAML document to accept data with "/api/export/pdf/hosts"

a. There was no request body sent with curl b. No request body should be expected, as this is not defined in the RAML spec c. it says Use "/" .... but where? (I don't understand this at all..) d. I don't intend to populate "body" in RAML, because this method shouldn't require one

Thanks

pickworth commented 8 years ago

Okay, I think i was just misled due to the error message.... Oops!

By coincidence, I also had an error in my handler function, but the osprey message was throwing me off, i was looking in the wrong place!

This drove me crazy for days.. I'm so glad that we figured this out... Maybe the error message can be hidden somehow, as many API requests don't have bodies and this is pretty much normal behaviour right?. Or we could change the message so it is more meaningful ? Also, what does, */* refer to, out of curiosity?

Thanks!

blakeembrey commented 8 years ago

I'll make a patch with some better messaging and think on what will make it better. The message can definitely be hidden, it was added to avoid confusing as to why .body had nothing accessible when using Osprey (because it ignores the body when it's not defined). I didn't realize there's any middleware that will always access it and I considered accessing it a warning, but I'll remove it and improve the README.

The */* was referring to what you can put in RAML to support any and all content types.

body:
  */*:

Thanks for pointing out some things that can be improved.