scottie1984 / swagger-ui-express

Adds middleware to your express app to serve the Swagger UI bound to your Swagger document. This acts as living documentation for your API hosted from within your app.
MIT License
1.43k stars 226 forks source link

Race condition for load balanced environments #178

Closed rfarris closed 2 years ago

rfarris commented 4 years ago

Issue:
Request to /swagger-ui-init.js randomly returns no data. It is really bad after a deploy, then gets better over time. Frequency of the issue depends on the size of cluster being load balanced.

Setup

express.use('/docs', swaggerUi.serve, swaggerUi.setup(null, {
  swaggerOptions: {
    validatorUrl: false,
    defaultModelsExpandDepth: 0,
    url: specUrl
  },
  customCssUrl: "/stylesheets/api_docs.css"
}));

Cause The swaggerInit global variable used in index.js does not get initialized until a request hits the server that goes all the way through the middleware chain and hits the swaggerUi.setup(..) middleware.

Repo Can be trivially reproduced by simply directly requesting the /swagger-ui-init.js url after starting up node.

or

  1. Restart cluster.
  2. Initial page load request hits Server 1. This does not match any of the paths being checked for in the middleware swaggerUi.serve and will continue the middleware chain until calling swaggerUi.setup(...). This returns the HTML and sets the swaggerInit global variable.
  3. Subsequent request for swagger-ui-init.js goes to Server 2. swaggerUi.serve matches the url /swagger-ui-init.js and returns the uninitialized swaggerInit global variable.
function swaggerInitFn(req, res, next) {
  if (req.url === '/package.json') {
    res.sendStatus(404)
  } else if (req.url === '/swagger-ui-init.js') {
    res.set('Content-Type', 'application/javascript')
    res.send(swaggerInit)
  } else {
    next()
  }
}

Workaround Force initialization at startup using a fake request and response.

const swaggerSetupMiddleware = swaggerUi.setup(null, {
  swaggerOptions: {
    validatorUrl: false,
    defaultModelsExpandDepth: 0,
    url: specUrl
  },
  customCssUrl: "/stylesheets/api_docs.css"
});
swaggerSetupMiddleware({}, {send: () => {}}); // force the docs to initialize.
express.use('/docs', swaggerUi.serve, swaggerSetupMiddleware);
pharindoko commented 4 years ago

I can confirm that this works.

const swaggerUi = require('swagger-ui-express');
...
      const swaggerSetupMiddleware = swaggerUi.setup(this.swaggerSpec);
      swaggerSetupMiddleware({}, { send: () => {} }, function(err: any): any {
        console.log(err);
      });

but I have no freakin clue how to write the request / response in typescript :-/


import swaggerUi as * from 'swagger-ui-express'
      const swaggerSetupMiddleware = swaggerUi.setup(this.swaggerSpec);
      swaggerSetupMiddleware({}, { send: () => {} }, function(err: any): any {
        console.log(err);
      });

rfarris commented 4 years ago

As of about 2 months ago when I last looked at them, the Typescript types for this package were broken machine generated garbage that prevented setting any options on setup. Unless someone's started maintaining them, they probably shouldn't be used.

pharindoko commented 4 years ago

As of about 2 months ago when I last looked at them, the Typescript types for this package were broken machine generated garbage that prevented setting any options on setup. Unless someone's started maintaining them, they probably shouldn't be used.

Lol yeah that‘s correct the types are useless. Comes from the express serve static core package:

https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/express-serve-static-core

robgraeber commented 4 years ago

Thanks for the workaround, the way you write it in Typescript is:

const swaggerUIMiddleware = SwaggerUI.setup(openApiDocumentation);
const req: any = {};
const res: any = { send: () => {} };

// Make a mock request to the swagger ui middleware to initialize it.
// Workaround issue: https://github.com/scottie1984/swagger-ui-express/issues/178
swaggerUIMiddleware(req, res, () => {});
app.use('/api-docs', SwaggerUI.serve, swaggerUIMiddleware);
pharindoko commented 4 years ago

@robgraeber aaaaaaaah ;) thanks - works ! 🥇

ianendboss commented 4 years ago

Thank you for opening up this issue! Jesus Christ, I spent two days trying to figure this out and finally had the thought that it might be a typescript issue.

Thanks @robgraeber @pharindoko

yinglejia commented 4 years ago

Hitting this bug in our apps too. @scottie1984 it would be great if you can fix this in swagger-ui-express so consumers don't have to workaround this in every app...

JoeNg93 commented 4 years ago

Halo, I attempted to fix the problem with this PR here https://github.com/scottie1984/swagger-ui-express/pull/190. The idea is to scope the swaggerInit instead of using it globally, hence remove the need of /swagger-ui-init.js

TurtleTower commented 4 years ago

Thank you so much!!

scottie1984 commented 3 years ago

Please review this #228 and let me know if it fixes.

scottie1984 commented 2 years ago

I will close this as I reverted the code that was causing this issue.