apigee-127 / swagger-tools

A Node.js and browser module that provides tooling around Swagger.
MIT License
702 stars 373 forks source link

Swagger route forces you to have an handler #274

Closed Mattzr closed 9 years ago

Mattzr commented 9 years ago

Hi,

Since I upgraded to 0.9.0 - I am getting an error when I try to request different endpoints:

{ "message": "Cannot resolve the configured swagger-router handler: getTransactionById" }

getTransactionById is my operation but I haven't defined any x-swagger-router-controller as I am using another module for handling that (swagger-pipes).

I think the developer should be free to have or not a controller to handler a request (e.g: return an error only if x-swagger-router-controller is defined and the handler couldn't be found)

reference: +* Updated swagger-router to throw a 500 when there is a configured route handler but it is missing (Issue #155)

whitlockjc commented 9 years ago

So you're using swagger-router but you're not specifying the x-swagger-router-controller for an operation and you're seeing the issue above?

Mattzr commented 9 years ago

exactly

whitlockjc commented 9 years ago

I think #218 causes this. Basically in #218, the idea was that you could use operationId only to do routing. I guess when I accepted that I didn't realize that it leaves you with an inability to not use swagger-router. I need to think about this because there really is no way to handle both use cases.

whitlockjc commented 9 years ago

@bachp Do you mind commenting on this?

bachp commented 9 years ago

I'm not sure if I understand right. But how would this have worked before the change?

As I understand it you either have to pass a controller yourself or you have to specify the controller to use via x-swagger-router-controller. Do I have a wrong view here?

whitlockjc commented 9 years ago

But doesn't #218 make it so that you can do routing without x-swagger-router-controller and it can be done by operationId alone? The way it use to work is if you didn't specify x-swagger-router-controller, we didn't try to do any routing but now, we will try to do it based on operationId even without x-swagger-router-controller.

bachp commented 9 years ago

Ah ok, I understand the issue. Maybe it can be changed to only do the routing if a router object is given. This means it would only try to do routing based on operationId if a controller exists and if not do nothing.

Would this work?

whitlockjc commented 9 years ago

But you'd still have no way to no if the user intended for there to be no routing done or the router requested does not exist whenever you do not use x-swagger-router-controller right? I could be wrong so that's why I'm asking instead of telling. Right now, missing handler detection is a feature of sorts and that feature cannot really work with #218. Maybe that feature doesn't matter?

bachp commented 9 years ago

I don't see why it was different before. I mean as far as I understand the getHandler function returns a value even if x-swagger-router-controller is not set. See Line 47 of the old version. If only operations Id is set it would just return _operationId.

I don't see the case where no routing should be done. I mean if you don't want to do routing, don't use the router middleware?

whitlockjc commented 9 years ago

Interesting...the code definitely says one thing but the user is reporting another. I definitely don't see how this could had worked like the user wanted ever. Maybe it's time we implement a x-swagger-router-ignore or something to allow you to use swagger-router but turn off its mapping for routes that the user doesn't want them. Thoughts?

bachp commented 9 years ago

What other features does swagger-router provide besides the mapping?

whitlockjc commented 9 years ago

Mocking. If you use swagger-router and enable mock mode, you can get generated responses based on your Swagger document.

bachp commented 9 years ago

@Mattzr Could you maybe explain a bit more about your use case?

Mattzr commented 9 years ago

Basically I am using swagger-node which is using swagger tool behind the scene. It does the job great for parameter validations and so on. However - for route handling - I was using https://github.com/apigee-127/bagpipes. It allows to create different chunk for reusable codes and hook them up into a pipeline. Therefore I didn't need the controller. But the new version of swagger-tool was stopping any incoming requests because it couldnt find its controller that wasn't specifiied intentionnally ! I reverted to fix that issue. (By the time - swagger pipe got upgraded and the new version define 'pipe' programmatically rather than in the swagger file as a new header)

spacesuitdiver commented 9 years ago

Is there a solution to this? I am just getting started with using Sails and running into this issue. I have a UserController using basic Blueprint routes and get Cannot resolve the configured swagger-router handler: UserController_create.

whitlockjc commented 9 years ago

What if we disable the handling that throws an error whenever a handler can't be found and instead logs it and sends the request downstream? Without being able to tell the difference between having no controller or having a missing controller?

@LeBlaaanc Your issue seems different because it seems like you could be specifying x-swagger-router-controller somehow because without it, the handler would look for create by itself. If you are indeed setting x-swagger-router-controller to UserController, then you need to create a UserController.js in your swagger-node controllers directory.

spacesuitdiver commented 9 years ago

@whitlockjc thanks for the response. It's the "magic" crud methods (rest blueprints) that Blueprint offers that are causing the issue I think as UserController#create isn't written into the controller explicitly. Jeremy mentioned to CC you @theganyo on IRC :)

whitlockjc commented 9 years ago

I would expect for sails that if we are relying on sails blueprints and router, we wouldn't even wire in the router support. But if we need it for something, basically we're at a point where we need to stop returning an error for a missing router handler and send the request downstream. I think that would be the only suitable solution. Since we log this in the debug output, missed handlers that is, it shouldn't be a big deal.

theganyo commented 9 years ago

Sails Blueprints and Swagger Node are probably going to be at odds with each other as they both attempt to handle requests. For any given path, you'd need to choose one of the other.

whitlockjc commented 9 years ago

We originally worked like this but back in #155 we changed it from falling through to explicitly throwing an error. I wonder if @Nepoxx has an opinion since this was their request.

What about making this configurable? That seems like the best option in my opinion. Then you can go either way by changing a configuration instead of having to get everyone to agree or not.

whitlockjc commented 9 years ago

So, what I did was I created a swagger-router option (ignoreMissingHandlers) that defaults to false to behave how it has since #155 but if you set it to true, the request will fall through to the next middleware in the middleware chain.

yasirbam commented 8 years ago

Hi, I am getting this error after upgrading to 0.9.0 I am not using "x-swagger-router-controller", I am just using it as a mock.

How do you set the swagger-router option (ignoreMissingHandlers) to true? thanks

whitlockjc commented 8 years ago

app.use(middleware.swaggerRouter({ignoreMissingHandlers: true}));

yasirbam commented 8 years ago

I have added the line to index.js but no luck Could the following line in index.js overriding this?

useStubs: process.env.NODE_ENV === 'development' ? true : false // Conditionally turn on stubs (mock mode)

Error: Cannot resolve the configured swagger-router handler: addTest<br> &nbsp; &nbsp;at swaggerRouter (

yasirbam commented 8 years ago

yes, setting the environment variable to something other than development worked. thanks

Kynde commented 8 years ago

The editor.swagger.io with full petstore generated node.js server faults with this error, too. Adding that "ignoreMissingHandlers" does not help.

Edit: Got it working. Some mixup with naming and the editor leaves the yaml/json a little lacking. Adding x-swagger-router-controller to point to the files split by tags and operationId to the functions helped. Sorry for the noise, but the point still stands that the generated node server defaults from editor.swagger.io yaml/json do not work currently.

kbouh320 commented 8 years ago

Hello,

@Kynde I currently have the same issue with editor.swagger.io, Error: Cannot resolve the configured swagger-router handler: undefined

Could you please describe what you did to correct it with generated files ? Thank you

Kynde commented 8 years ago

I had:

  /version:
    get:
      tags:
      - "misc"
      parameters: []
...

And thus it created controllers/Misc.js with the versionGet in it. So I added the x-swagger-route-controller to locate the Misc and operationId to find the function like this:

  /version:
    get:
      x-swagger-router-controller: Misc
      operationId: versionGet
      tags:
      - "misc"
      parameters: []
...
kbouh320 commented 8 years ago

Perfect,

Thank you @Kynde

imran-uk commented 8 years ago

@Kynde thanks, your solution worked for us too.

jordanwalsh23 commented 8 years ago

Thanks @Kynde. Helped me out as well.. Great stuff.

girishbr commented 8 years ago

@Kynde didn't work for me

This

consumes: 
  - "application/json"
produces: 
  - "application/json"
x-a127-config: {}
x-a127-services: {}
paths: 
  /projects: 
    get: 
      x-swagger-router-controller: "project"    
      description: "Returns projects for the user present in the cookie"
      operationId: "getProjectsForUser"
      tags:
      - "projects"      
      responses: 
        200: 
          description: "Success"
          schema: 
            $ref: "#/definitions/openResponse"
        500: 
          description: "Error"
          schema: 
            $ref: "#/definitions/ErrorResponse"

throws

{
  "message": "Cannot resolve the configured swagger-router handler: project_getProjectsForUser"
}

controllers/project.js has that exact method

module.exports = {
    getProjectsForUser: getProjectsForUser,...}
Kynde commented 8 years ago

Your x-swagger-router-controller has project while the tags says projects in plural. Try fixing that.

On Thu, 24 Dec 2015 07:32 girishbr notifications@github.com wrote:

@Kynde https://github.com/Kynde didn't work for me

This

consumes:

  • "application/json" produces:
  • "application/json" x-a127-config: {} x-a127-services: {} paths: /projects: get: x-swagger-router-controller: "project" description: "Returns projects for the user present in the cookie" operationId: "getProjectsForUser" tags:
    • "projects" responses: 200: description: "Success" schema: $ref: "#/definitions/openResponse" 500: description: "Error" schema: $ref: "#/definitions/ErrorResponse"

throws

{ "message": "Cannot resolve the configured swagger-router handler: project_getProjectsForUser" }

controllers/project.js has that exact method

module.exports = { getProjectsForUser: getProjectsForUser,...}

— Reply to this email directly or view it on GitHub https://github.com/apigee-127/swagger-tools/issues/274#issuecomment-167044801 .