Invalid type error with arrays and restify

marcobrinkmann commented 8 years ago

I created a new project with the restify option of the command line tool:

swagger project create petrestify

I used the petstore example specification as template and added a controller to handle listPets operation:

https://gist.github.com/marcobrinkmann/4aed8b6a972c6e6a4224

The controller looks like:

module.exports = {
  listPets: function(req, res) {
    res.json([{
      id: 1,
      name: 'Dobermann',
      tag: 'Dogs'
    }]);
  }
};

Added a simple test:

it('should return a array with pets', function(done) {
  request(server)
    .get('/v1/pets')
    .set('Accept', 'application/json')
    .expect('Content-Type', /json/)
    .expect(200)
    .end(function(err, res) {
      should.not.exist(err);

      res.body.should.have.length(1);
      res.body[0].name.should.eql('Dobermann');
      done();
  });
});

The test passes as expected, but the schema does not get validated. From the swagger debug mode:

swagger-tools:middleware:router GET /v1/pets +0ms
swagger-tools:middleware:router   Will process: yes +0ms
swagger-tools:middleware:router   Route handler: pets_listPets +1ms
swagger-tools:middleware:router     Missing: no +0ms
swagger-tools:middleware:router     Ignored: no +0ms
swagger-tools:middleware:router     Using mock: no +0ms
swagger-tools:middleware:validator   Response validation: failed +5ms
swagger-tools:middleware:validator   Reason: Failed schema validation +0ms
swagger-tools:middleware:validator   Errors: +0ms
swagger-tools:middleware:validator     0: +0ms
swagger-tools:middleware:validator       code: INVALID_TYPE +0ms
swagger-tools:middleware:validator       message: Expected type array but found type undefined +0ms
swagger-tools:middleware:validator       path: [] +0ms
swagger-tools:middleware:validator   Stack: +1ms
swagger-tools:middleware:validator       at throwErrorWithCode (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:121:13) +0ms
swagger-tools:middleware:validator       at Object.module.exports.validateAgainstSchema (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:175:7) +0ms
swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/middleware/swagger-validator.js:141:22 +0ms
swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:358:13 +0ms
swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:239:13 +0ms
swagger-tools:middleware:validator       at _arrayEach (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:91:13) +1ms
swagger-tools:middleware:validator       at _each (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:82:13) +0ms
swagger-tools:middleware:validator       at async.forEachOf.async.eachOf (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:238:9) +0ms
swagger-tools:middleware:validator       at _asyncMap (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:357:9) +0ms
swagger-tools:middleware:validator       at Object.map (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:340:20) +0ms

I tested also the validation of simple objects and I don't have any issues. I just can't get it working with arrays (tried different responses res.send(), res.json() and different specs). I debugged it for a while a saw that the z-schema validate function doesn't have a json object, so I tried the same setup with express. All steps done exactly the same, except I selected the express option in the command line tool. Surprisingly it works like a charm. Thanks for looking into this issue.

whitlockjc commented 8 years ago

Could you try setting the response code to 200 prior to sending your response?

marcobrinkmann commented 8 years ago

Same behaviour with all variations:

res.status(200);
res.send([{...

res.send(200, [{...

res.status(200);
res.json([{...

whitlockjc commented 8 years ago

Okay, just checking. We look up the response schema based on that value so if it's not set, we would go to default and that could cause an issue. What version of swagger-tools is installed? npm ls | grep "swagger-tools" The reason I ask is we should be outputting which response schema we're using to validate in the latest version.

marcobrinkmann commented 8 years ago

It's swagger-tools@0.9.7

whitlockjc commented 8 years ago

Are you missing any of your debug output? I would expect to see some Response code: {code || default} in the debug output.

marcobrinkmann commented 8 years ago

Full log for restify setup:

DEBUG=swagger-tools:middleware:* swagger project start

Starting: ../petrestify/app.js...
  project started here: http://localhost:10010/v1
  project will restart on changes.
  to restart at any time, enter `rs`
  swagger-tools:middleware:router Initializing swagger-router middleware +0ms
  swagger-tools:middleware:router   Mock mode: disabled +2ms
  swagger-tools:middleware:router   Controllers: +1ms
  swagger-tools:middleware:router     ../petrestify/api/controllers/pets.js: +1ms
  swagger-tools:middleware:router       pets_listPets +0ms
  swagger-tools:middleware:validator Initializing swagger-validator middleware +1ms
  swagger-tools:middleware:validator   Response validation: enabled +0ms
  swagger-tools:middleware:security Initializing swagger-security middleware +4ms
  swagger-tools:middleware:security   Security handlers: 0 +1ms
  swagger-tools:middleware:metadata Initializing swagger-metadata middleware +80ms
  swagger-tools:middleware:metadata   Identified Swagger version: 2.0 +1ms
  swagger-tools:middleware:metadata   Found Path: /pets +1ms
  swagger-tools:middleware:metadata   Found Path: /pets/{petId} +1ms
  swagger-tools:middleware:metadata Initializing swagger-metadata middleware +2ms
  swagger-tools:middleware:metadata   Identified Swagger version: 2.0 +0ms
  swagger-tools:middleware:metadata   Found Path: /pets +1ms
  swagger-tools:middleware:metadata   Found Path: /pets/{petId} +0ms
  swagger-tools:middleware:metadata GET /v1/pets +18s
  swagger-tools:middleware:metadata   Is a Swagger path: true +1ms
  swagger-tools:middleware:metadata   Is a Swagger operation: true +0ms
  swagger-tools:middleware:metadata   Processing Parameters +0ms
  swagger-tools:middleware:metadata     limit +1ms
  swagger-tools:middleware:metadata       Type: integer (format: int32) +0ms
  swagger-tools:middleware:metadata       Value provided: false +0ms
  swagger-tools:middleware:metadata       Value: undefined +1ms
  swagger-tools:middleware:security GET /v1/pets +2ms
  swagger-tools:middleware:security   Will process: yes +0ms
  swagger-tools:middleware:validator GET /v1/pets +0ms
  swagger-tools:middleware:validator   Will process: yes +1ms
  swagger-tools:middleware:validator   Request validation: succeeded +0ms
  swagger-tools:middleware:router GET /v1/pets +1ms
  swagger-tools:middleware:router   Will process: yes +0ms
  swagger-tools:middleware:router   Route handler: pets_listPets +0ms
  swagger-tools:middleware:router     Missing: no +0ms
  swagger-tools:middleware:router     Ignored: no +0ms
  swagger-tools:middleware:router     Using mock: no +0ms
  swagger-tools:middleware:validator   Response validation: failed +7ms
  swagger-tools:middleware:validator   Reason: Failed schema validation +0ms
  swagger-tools:middleware:validator   Errors: +0ms
  swagger-tools:middleware:validator     0: +0ms
  swagger-tools:middleware:validator       code: INVALID_TYPE +0ms
  swagger-tools:middleware:validator       message: Expected type array but found type undefined +0ms
  swagger-tools:middleware:validator       path: [] +0ms
  swagger-tools:middleware:validator   Stack: +1ms
  swagger-tools:middleware:validator       at throwErrorWithCode (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:121:13) +0ms
  swagger-tools:middleware:validator       at Object.module.exports.validateAgainstSchema (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:175:7) +0ms
  swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/middleware/swagger-validator.js:141:22 +0ms
  swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:358:13 +0ms
  swagger-tools:middleware:validator       at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:239:13 +0ms
  swagger-tools:middleware:validator       at _arrayEach (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:91:13) +0ms
  swagger-tools:middleware:validator       at _each (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:82:13) +0ms
  swagger-tools:middleware:validator       at async.forEachOf.async.eachOf (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:238:9) +0ms
  swagger-tools:middleware:validator       at _asyncMap (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:357:9) +0ms
  swagger-tools:middleware:validator       at Object.map (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:340:20) +0ms
Error: Response validation failed: failed schema validation
    at throwErrorWithCode (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:121:13)
    at Object.module.exports.validateAgainstSchema (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/lib/validators.js:175:7)
    at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/middleware/swagger-validator.js:141:22
    at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:358:13
    at ../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:239:13
    at _arrayEach (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:91:13)
    at _each (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:82:13)
    at async.forEachOf.async.eachOf (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:238:9)
    at _asyncMap (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:357:9)
    at Object.map (../petrestify/node_modules/swagger-restify-mw/node_modules/swagger-node-runner/node_modules/swagger-tools/node_modules/async/lib/async.js:340:20)

The express version looks like:

Starting: ../petexpress/app.js...
  project started here: http://localhost:10010/v1
  project will restart on changes.
  to restart at any time, enter `rs`
  swagger-tools:middleware:router Initializing swagger-router middleware +0ms
  swagger-tools:middleware:router   Mock mode: disabled +2ms
  swagger-tools:middleware:router   Controllers: +1ms
  swagger-tools:middleware:router     ../petexpress/api/controllers/pet.js: +1ms
  swagger-tools:middleware:router       pet_listPets +0ms
  swagger-tools:middleware:validator Initializing swagger-validator middleware +2ms
  swagger-tools:middleware:validator   Response validation: enabled +0ms
  swagger-tools:middleware:security Initializing swagger-security middleware +8ms
  swagger-tools:middleware:security   Security handlers: 0 +0ms
  swagger-tools:middleware:metadata Initializing swagger-metadata middleware +139ms
  swagger-tools:middleware:metadata   Identified Swagger version: 2.0 +0ms
  swagger-tools:middleware:metadata   Found Path: /pets +2ms
  swagger-tools:middleware:metadata   Found Path: /pets/{petId} +1ms
  swagger-tools:middleware:metadata GET /v1/pets +4s
  swagger-tools:middleware:metadata   Is a Swagger path: true +0ms
  swagger-tools:middleware:metadata   Is a Swagger operation: true +0ms
  swagger-tools:middleware:metadata   Processing Parameters +1ms
  swagger-tools:middleware:metadata     limit +0ms
  swagger-tools:middleware:metadata       Type: integer (format: int32) +0ms
  swagger-tools:middleware:metadata       Value provided: false +1ms
  swagger-tools:middleware:metadata       Value: undefined +0ms
  swagger-tools:middleware:security GET /v1/pets +2ms
  swagger-tools:middleware:security   Will process: yes +0ms
  swagger-tools:middleware:validator GET /v1/pets +1ms
  swagger-tools:middleware:validator   Will process: yes +0ms
  swagger-tools:middleware:validator   Request validation: succeeded +0ms
  swagger-tools:middleware:router GET /v1/pets +1ms
  swagger-tools:middleware:router   Will process: yes +0ms
  swagger-tools:middleware:router   Route handler: pet_listPets +0ms
  swagger-tools:middleware:router     Missing: no +0ms
  swagger-tools:middleware:router     Ignored: no +0ms
  swagger-tools:middleware:router     Using mock: no +0ms
  swagger-tools:middleware:validator   Response validation: succeeded +4ms

whitlockjc commented 8 years ago

I'm still not seeing Response code: {code || default} in your output. I'm trying to trace the code to see if I can figure out why. I ran the local test suite for swagger-tools and response validation failures had debug output with the code being used in it as expected.

whitlockjc commented 8 years ago

The reason why this is important is I'm thinking the wrong schema is being used to validate the response. Could you throw some debugging in your route handler for me to let me know what the status code is right before you call res.json?

marcobrinkmann commented 8 years ago

This is what I found out using node 4.2.1: In restify a res.json() command is truly executed as:

// restify/lib/response.js

Response.prototype.send = function send(code, body, headers) {
    // ...
    function _cb(err, _body) {
        // ...
        if (self._data && !(isHead || code === 204 || code === 304)) {
            self.write(self._data);
        }

        self.end();
    }

    // ...
    this.format(body, _cb);

The combination of res.write() and res.end() is causing the problem. The written data will never be received by the swagger validator middleware:

// swagger-tools/middleware/swagger-validator.js

var wrapEnd = function (req, res, next) {
    // ...
    res.end = function (data, encoding) {
        // Never receives any data from Response.send() in restify

The node.js documentation states that the combination of response.write() and response.end() is equivalent to response.end(data, encoding). For testing purposes I extended the restify function to self.end(self._data) and surprisingly this works. I looked into the expressjs response function and they use also the response.end(data, encoding) syntax.

This is how far I can debug it. Hope this analysis helps to figure out what to do.

whitlockjc commented 8 years ago

Thanks for your help. Wrapping res.end seemed like a pretty solid approach but I guess we'll need to do more since Restify seems to do things differently.

eckdanny commented 8 years ago

^ helpful. I'm learning the swagger spec while hacking on a project using swagger-tools@0.9.16 (and swagger-resitfy-mw@0.1.0). Had similar headaches as @marcobrinkmann with type: array throwing Response validation errors. Took a while to find this thread since I suspected my n00b-level grasp of the swagger spec was the problem ¯_(ツ)_/¯

fairsayan commented 8 years ago

any plan to fix this issue? I'm experimenting the same problem.

jonkr commented 8 years ago

Me too!

sherpya commented 8 years ago

res.header('content-type', 'application/json');
return res.end(results);

does work, while

res.json(results)

not

do you have a quick workaround?

jackkelly-optimizely commented 8 years ago

Anyone have any progress/notes on this? I've run into this exact issue and any help would be appreciated :)

DonutEspresso commented 7 years ago

Hello, restify maintainer here. Can someone help me understand what the swagger validation workflow is, and how it is expected to work (does it hook into res end event, or..)? I'd like to fully understand the underlying issue here before making any changes.

revnode commented 7 years ago

@DonutEspresso I think these are the interesting bits as mentioned by @marcobrinkmann

https://github.com/apigee-127/swagger-tools/blob/master/middleware/swagger-validator.js

Lines 337-340 and then the wrapEnd function itself at 172.

var wrapEnd = function (req, res, next) {
  var operation = req.swagger.operation;
  var originalEnd = res.end;
  var vPath = _.cloneDeep(req.swagger.operationPath);
  var swaggerVersion = req.swagger.swaggerVersion;

  var writtenData = [];

  var originalWrite = res.write;
  res.write = function(data) {
    if(typeof data !== 'undefined') {
      writtenData.push(data);
    }
    // Don't call the originalWrite. We want to validate the data before writing
    // it to our response.
  };

  res.end = function (data, encoding) {
    var schema = operation;
    var val;
    if(data)
    {
      if(data instanceof Buffer) {
        writtenData.push(data);
        val = Buffer.concat(writtenData);
      }
      else if(data instanceof String) {
        writtenData.push(new Buffer(data));
        val = Buffer.concat(writtenData);
      }
      else {
        val = data;
      }
    }
    else if(writtenData.length !== 0) {
      val = Buffer.concat(writtenData);
    }

    var responseCode;

    // Replace 'res.end' and 'res.write' with the originals
    res.write = originalWrite;
    res.end = originalEnd;

    debug('  Response validation:');

    // If the data is a buffer, convert it to a string so we can parse it prior to validation
    if (val instanceof Buffer) {
      val = val.toString(encoding);
    }

    // Express removes the Content-Type header from 204/304 responses which makes response validation impossible
    if (_.isUndefined(res.getHeader('content-type')) && [204, 304].indexOf(res.statusCode) > -1) {
      sendData(swaggerVersion, res, val, encoding, true);
      return; // do NOT call next() here, doing so would execute remaining middleware chain twice
    }

    try {
      // Validate the content type
      try {
        validators.validateContentType(req.swagger.apiDeclaration ?
                                         req.swagger.apiDeclaration.produces :
                                         req.swagger.swaggerObject.produces,
                                       operation.produces, res);
      } catch (err) {
        err.failedValidation = true;

        throw err;
      }

      if (_.isUndefined(schema.type)) {
        if (schema.schema) {
          schema = schema.schema;
        } else if (swaggerVersion === '1.2') {
          schema = _.find(operation.responseMessages, function (responseMessage, index) {
            if (responseMessage.code === res.statusCode) {
              vPath.push('responseMessages', index.toString());

              responseCode = responseMessage.code;

              return true;
            }
          });

          if (!_.isUndefined(schema)) {
            schema = schema.responseModel;
          }
        } else {
          schema = _.find(operation.responses, function (response, code) {
            if (code === (res.statusCode || 200).toString()) {
              vPath.push('responses', code);

              responseCode = code;

              return true;
            }
          });

          if (_.isUndefined(schema) && operation.responses.default) {
            responseCode = 'default';
            schema = operation.responses.default;

            vPath.push('responses', 'default');
          }
        }
      }

      debug('    Response ' + (swaggerVersion === '1.2' ? 'message' : 'code') + ': ' + responseCode);

      if (_.isUndefined(schema)) {
        sendData(swaggerVersion, res, val, encoding, true);
      } else {
        validateValue(req, schema, vPath, val, function (err) {
          if (err) {
            throw err;
          }
          sendData(swaggerVersion, res, val, encoding, true);
        });
      }
    } catch (err) {
      if (err.failedValidation) {
        err.originalResponse = data;
        err.message = 'Response validation failed: ' + err.message.charAt(0).toLowerCase() + err.message.substring(1);

        debug('    Validation: failed');

        mHelpers.debugError(err, debug);
      }

      return next(err);
    }
  };
};

DonutEspresso commented 7 years ago

Ah, so swagger is monkey patching res.write/res.end underneath us. The linked code here appears to buffer up writtenData on res.write and then use it when called by res.end for validation. If validation isn't occurring at that point, I'm not sure there's much we (restify) can do, since we're no longer in control. Am I missing something?

devcog commented 7 years ago

Hitting the same issue. Glad I founds this thread. Ended up just calling res.header and res.end like @sherpya suggested. Any plans to fix in swagger-node ?

swagger-api / swagger-node

Invalid type error with arrays and restify #313