Skip to content

Commit

Permalink
Merge pull request #782 from postmanlabs/feature/report-usererror-inv…
Browse files Browse the repository at this point in the history 
…alid-definitions

Added support for reporting UserErrors in case when OpenAPI definition to converted is invalid.
  • Loading branch information
@VShingala
VShingala committed Feb 1, 2024
2 parents 7f7dff4 + d9f02a2 commit dce4823
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 67 deletions.
8 changes: 5 additions & 3 deletions index.js
View file
Expand Up @@ -3,7 +3,9 @@
const { MODULE_VERSION } = require('./lib/schemapack.js');

const _ = require('lodash'),
SchemaPack = require('./lib/schemapack.js').SchemaPack;
SchemaPack = require('./lib/schemapack.js').SchemaPack,
UserError = require('./lib/common/UserError'),
DEFAULT_INVALID_ERROR = 'Provided definition is invalid';

module.exports = {
// Old API wrapping the new API
Expand All @@ -13,7 +15,7 @@ module.exports = {
if (schema.validated) {
return schema.convert(cb);
}
return cb(null, schema.validationResult);
return cb(new UserError(_.get(schema, 'validationResult.reason', DEFAULT_INVALID_ERROR)));
},

convertV2: function(input, options, cb) {
Expand All @@ -23,7 +25,7 @@ module.exports = {
return schema.convertV2(cb);
}

return cb(null, schema.validationResult);
return cb(new UserError(_.get(schema, 'validationResult.reason', DEFAULT_INVALID_ERROR)));
},

validate: function (input) {
Expand Down
70 changes: 36 additions & 34 deletions test/unit/base.test.js
View file
Expand Up @@ -993,30 +993,30 @@ describe('CONVERT FUNCTION TESTS ', function() {
});
});

it('should not return undefined in the error message if spec is not valid JSON/YAML', function(done) {
it('should return correct error message if spec is not valid JSON/YAML', function(done) {
// invalid JSON
Converter.convert({ type: 'string', data: '{"key": { "value" : } ' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('undefined');
Converter.convert({ type: 'string', data: '{"key": { "value" : } ' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.include('Invalid format. Input must be in YAML or JSON format.');
});

// invalid YAML
Converter.convert({ type: 'string', data: ' :' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('undefined');
Converter.convert({ type: 'string', data: ' :' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.include('Invalid format. Input must be in YAML or JSON format.');
done();
});
});

it('should throw an invalid format error and not semantic version missing error when yaml.safeLoad ' +
'does not throw an error while parsing yaml', function(done) {
// YAML for which yaml.safeLoad does not throw an error
Converter.convert({ type: 'string', data: 'no error yaml' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('Specification must contain a semantic version number' +
Converter.convert({ type: 'string', data: 'no error yaml' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.not.include('Specification must contain a semantic version number' +
' of the OAS specification');
done();
});
Expand Down Expand Up @@ -1349,10 +1349,10 @@ describe('CONVERT FUNCTION TESTS ', function() {
it('The converter must throw an error for invalid null info', function (done) {
var openapi = fs.readFileSync(invalidNullInfo, 'utf8');
Converter.convert({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain an Info Object for the meta-data of the API');
done();
});
Expand All @@ -1361,10 +1361,10 @@ describe('CONVERT FUNCTION TESTS ', function() {
it('The converter must throw an error for invalid null info title', function (done) {
var openapi = fs.readFileSync(invalidNullInfoTitle, 'utf8');
Converter.convert({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain a title in order to generate a collection');
done();
});
Expand All @@ -1373,10 +1373,10 @@ describe('CONVERT FUNCTION TESTS ', function() {
it('The converter must throw an error for invalid null info version', function (done) {
var openapi = fs.readFileSync(invalidNullInfoVersion, 'utf8');
Converter.convert({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain a semantic version number of the API in the Info Object');
done();
});
Expand Down Expand Up @@ -2408,9 +2408,9 @@ describe('INTERFACE FUNCTION TESTS ', function () {
validationResult = Converter.validate({ type: 'string', data: openapi });

expect(validationResult.result).to.equal(false);
Converter.convert({ type: 'string', data: openapi }, {}, function(err, conversionResult) {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
Converter.convert({ type: 'string', data: openapi }, {}, function(err) {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
done();
});
});
Expand All @@ -2422,9 +2422,10 @@ describe('INTERFACE FUNCTION TESTS ', function () {
var result = Converter.validate({ type: 'fil', data: 'invalid_path' });
expect(result.result).to.equal(false);
expect(result.reason).to.contain('input');
Converter.convert({ type: 'fil', data: 'invalid_path' }, {}, function(err, conversionResult) {
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason).to.equal('Invalid input type (fil). type must be one of file/json/string.');
Converter.convert({ type: 'fil', data: 'invalid_path' }, {}, function(err) {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.equal('Invalid input type (fil). type must be one of file/json/string.');
done();
});
});
Expand All @@ -2434,9 +2435,10 @@ describe('INTERFACE FUNCTION TESTS ', function () {
it('(type: file)', function(done) {
var result = Converter.validate({ type: 'file', data: 'invalid_path' });
expect(result.result).to.equal(false);
Converter.convert({ type: 'file', data: 'invalid_path' }, {}, function(err, result) {
expect(result.result).to.equal(false);
expect(result.reason).to.equal('ENOENT: no such file or directory, open \'invalid_path\'');
Converter.convert({ type: 'file', data: 'invalid_path' }, {}, function(err) {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.equal('ENOENT: no such file or directory, open \'invalid_path\'');
done();
});
});
Expand Down
11 changes: 6 additions & 5 deletions test/unit/bin.test.js
View file
Expand Up @@ -33,10 +33,11 @@ describe('openapi2postmanv2 ', function() {
});

it('should show appropriate messages for invalid input', function (done) {
exec('./bin/openapi2postmanv2.js -s test/data/invalid_openapi/multiple-components.yaml', function(err, stdout) {
expect(err).to.be.null;
expect(stdout).to.include('duplicated mapping key');
done();
});
exec('./bin/openapi2postmanv2.js -s test/data/invalid_openapi/multiple-components.yaml',
function(err, stdout, stderr) {
expect(err).to.be.null;
expect(stderr).to.include('duplicated mapping key');
done();
});
});
});
50 changes: 25 additions & 25 deletions test/unit/convertV2.test.js
View file
Expand Up @@ -947,30 +947,30 @@ describe('The convert v2 Function', function() {
});
});

it('should not return undefined in the error message if spec is not valid JSON/YAML', function(done) {
it('should return correct error message if spec is not valid JSON/YAML', function(done) {
// invalid JSON
Converter.convertV2({ type: 'string', data: '{"key": { "value" : } ' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('undefined');
Converter.convertV2({ type: 'string', data: '{"key": { "value" : } ' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.include('Invalid format. Input must be in YAML or JSON format.');
});

// invalid YAML
Converter.convertV2({ type: 'string', data: ' :' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('undefined');
Converter.convertV2({ type: 'string', data: ' :' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.include('Invalid format. Input must be in YAML or JSON format.');
done();
});
});

it('should throw an invalid format error and not semantic version missing error when yaml.safeLoad ' +
'does not throw an error while parsing yaml', function(done) {
// YAML for which yaml.safeLoad does not throw an error
Converter.convertV2({ type: 'string', data: 'no error yaml' }, {}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.be.false;
expect(conversionResult.reason).to.not.include('Specification must contain a semantic version number' +
Converter.convertV2({ type: 'string', data: 'no error yaml' }, {}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message).to.not.include('Specification must contain a semantic version number' +
' of the OAS specification');
done();
});
Expand Down Expand Up @@ -1244,10 +1244,10 @@ describe('The convert v2 Function', function() {
it('The converter must throw an error for invalid null info', function (done) {
var openapi = fs.readFileSync(invalidNullInfo, 'utf8');
Converter.convertV2({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain an Info Object for the meta-data of the API');
done();
});
Expand All @@ -1256,10 +1256,10 @@ describe('The convert v2 Function', function() {
it('The converter must throw an error for invalid null info title', function (done) {
var openapi = fs.readFileSync(invalidNullInfoTitle, 'utf8');
Converter.convertV2({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain a title in order to generate a collection');
done();
});
Expand All @@ -1268,10 +1268,10 @@ describe('The convert v2 Function', function() {
it('The converter must throw an error for invalid null info version', function (done) {
var openapi = fs.readFileSync(invalidNullInfoVersion, 'utf8');
Converter.convertV2({ type: 'string', data: openapi },
{}, (err, conversionResult) => {
expect(err).to.be.null;
expect(conversionResult.result).to.equal(false);
expect(conversionResult.reason)
{}, (err) => {
expect(err).to.not.be.null;
expect(err.name).to.eql('UserError');
expect(err.message)
.to.equal('Specification must contain a semantic version number of the API in the Info Object');
done();
});
Expand Down

0 comments on commit dce4823

Please sign in to comment.