Merge 61cc5da into 9c240aa

lib/index.js

@@ -1,45 +1,25 @@
 const parse = require('./parse');
 const compileFromApiElements = require('./compile');
 
-function createParserErrorCompilationResult(message) {
-  return {
-    mediaType: null,
-    transactions: [],
-    annotations: [{
-      type: 'error', component: 'apiDescriptionParser', message, location: [[0, 1]],
-    }],
-  };
-}
 
-function compile(source, filename, callback) {
-  // All regular parser-related or compilation-related annotations
-  // should be returned in the "compilation result". Callback should get
-  // an error only in case of unexpected crash.
+function compile(apiDescription, filename, callback) {
+  parse(apiDescription, (err, parseResult) => {
+    // Shouldn't happen, 'parse' turns all parser crashes into annotations
+    if (err) { callback(err); return; }
 
-  parse(source, (err, parseResult) => {
-    // If 'apiElements' isn't empty, then we don't need to care about 'err'
-    // as it should be represented by annotation inside 'apiElements'
-    // and compilation should be able to deal with it and to propagate it.
+    // Should always set annotations and never throw, try/catch deals only
+    // with unexpected compiler crashes
     let compilationResult;
-    if (!(parseResult ? parseResult.apiElements : undefined)) {
-      if (err) { return callback(null, createParserErrorCompilationResult(err.message)); }
-
-      const message = 'The API description parser was unable to provide a valid parse result';
-      return callback(null, createParserErrorCompilationResult(message));
-    }
-
-    // The try/catch is just to deal with unexpected crash. Compilation passes
-    // all errors as part of the 'result' and it should not throw anything
-    // in any case.
     try {
       const { mediaType, apiElements } = parseResult;
       compilationResult = compileFromApiElements(mediaType, apiElements, filename);
-    } catch (error) {
-      return callback(error);
+    } catch (syncErr) {
+      callback(syncErr);
+      return;
     }
-
-    return callback(null, compilationResult);
+    callback(null, compilationResult);
   });
 }
 
+
 module.exports = { compile };

lib/parse.js

@@ -1,56 +1,58 @@
 const fury = require('fury');
 
+
 fury.use(require('fury-adapter-apib-parser'));
 fury.use(require('fury-adapter-swagger'));
 fury.use(require('fury-adapter-oas3-parser'));
 
-function createWarning(message) {
-  const annotationElement = new fury.minim.elements.Annotation(message);
-  annotationElement.classes.push('warning');
-  annotationElement.attributes.set('sourceMap', [
-    new fury.minim.elements.SourceMap([[0, 1]]),
+const { Annotation, SourceMap, ParseResult } = fury.minim.elements;
+
+
+function createAnnotation(type, message) {
+  const element = new Annotation(message);
+  element.classes.push(type);
+  element.attributes.set('sourceMap', [
+    new SourceMap([[0, 1]]),
   ]);
-  return annotationElement;
+  return element;
 }
 
-function parse(source, callback) {
-  let mediaType;
-  let warningElement = null;
-  const adapters = fury.detect(source);
 
+function detectMediaType(apiDescription) {
+  const adapters = fury.detect(apiDescription);
   if (adapters.length) {
-    [mediaType] = adapters[0].mediaTypes;
-  } else {
-    mediaType = 'text/vnd.apiblueprint';
-    warningElement = createWarning(
-      'Could not recognize API description format.'
-      + ' Falling back to API Blueprint by default.'
-    );
+    return { mediaType: adapters[0].mediaTypes[0], fallback: false };
   }
+  return { mediaType: 'text/vnd.apiblueprint', fallback: true };
+}
 
-  const args = { source, mediaType, generateSourceMap: true };
 
-  return fury.parse(args, (err, apiElements) => {
-    let modifiedApiElements = apiElements;
-    let nativeError = null;
+function parse(apiDescription, callback) {
+  const { mediaType, fallback } = detectMediaType(apiDescription);
 
-    if (!(err || apiElements)) {
-      nativeError = new Error('Unexpected parser error occurred');
-    } else if (err) {
-      // Turning Fury error object into standard JavaScript error
-      nativeError = new Error(err.message);
-    } else if (apiElements && !apiElements.errors.isEmpty) {
-      nativeError = new Error('Parser finished with errors');
-    }
+  fury.parse({
+    source: apiDescription,
+    mediaType,
+    generateSourceMap: true,
+  }, (err, parseResult) => {
+    const apiElements = parseResult || new ParseResult([]);
 
-    if (modifiedApiElements) {
-      if (warningElement) { modifiedApiElements.unshift(warningElement); }
-    } else {
-      modifiedApiElements = null;
+    if (fallback) {
+      apiElements.unshift(createAnnotation('warning', (
+        'Could not recognize API description format, assuming API Blueprint'
+      )));
+    }
+    if (err && !parseResult) {
+      // The condition should be only 'if (err)'
+      // https://github.com/apiaryio/api-elements.js/issues/167
+      apiElements.unshift(createAnnotation('error', (
+        `Could not parse API description: ${err.message}`
+      )));
     }
 
-    return callback(nativeError, { mediaType, apiElements: modifiedApiElements });
+    callback(null, { mediaType, apiElements });
   });
 }
 
+
 module.exports = parse;

scripts/pretest.js

@@ -27,7 +27,7 @@ function parseFixture(fixturePath) {
   return new Promise((resolve, reject) => {
     const fixture = fs.readFileSync(fixturePath, { encoding: 'utf8' });
     parse(fixture, (err, result) => {
-      if (err && !result.apiElements) reject(err); // because of strange error handling in parse.js
+      if (err) reject(err);
       else resolve(result.apiElements);
     });
   }).then((apiElements) => {
@@ -48,4 +48,4 @@ console.log(`Parsing ${fixtures.length} fixtures...`);
 Promise
   .all(fixtures.map(parseFixture))
   .then(() => { console.log('Fixtures ready!'); })
-  .catch(console.error);
+  .catch((err) => { console.error(err); process.exitCode = 1; });

test/integration/dredd-transactions-test.js

@@ -44,7 +44,7 @@ describe('Dredd Transactions', () => {
     it('produces warning about falling back to API Blueprint', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
       type: 'warning',
       component: 'apiDescriptionParser',
-      message: 'to API Blueprint',
+      message: 'assuming API Blueprint',
     })));
   });
 
@@ -67,7 +67,7 @@ describe('Dredd Transactions', () => {
     it('produces warning about falling back to API Blueprint', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
       type: 'warning',
       component: 'apiDescriptionParser',
-      message: 'to API Blueprint',
+      message: 'assuming API Blueprint',
     })));
 
     it('produces a warning about the API Blueprint not being valid', () => assert.jsonSchema(compilationResult.annotations[1], createAnnotationSchema({
@@ -101,7 +101,7 @@ describe('Dredd Transactions', () => {
     it('produces a warning about falling back to API Blueprint', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
       type: 'warning',
       component: 'apiDescriptionParser',
-      message: 'to API Blueprint',
+      message: 'assuming API Blueprint',
     })));
   });
 
@@ -165,80 +165,22 @@ describe('Dredd Transactions', () => {
     });
   });
 
-  describe('when parser unexpectedly provides just error and no API Elements', () => {
-    const apiDescription = '... dummy API description document ...';
-    const message = '... dummy error message ...';
-    let compilationResult;
-
-    beforeEach((done) => {
-      const stubbedDreddTransactions = proxyquire('../../lib/index', {
-        './parse': (input, callback) => callback(new Error(message)),
-      });
-      stubbedDreddTransactions.compile(apiDescription, null, (err, result) => {
-        compilationResult = result;
-        done(err);
-      });
-    });
-
-    it('produces one annotation, no transactions', () => assert.jsonSchema(compilationResult, createCompilationResultSchema({
-      annotations: 1,
-      transactions: 0,
-    })));
-
-    it('turns the parser error into a valid annotation', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
-      type: 'error',
-      message,
-    })));
-  });
-
-  describe('when parser unexpectedly provides error and malformed API Elements', () => {
-    const apiDescription = '... dummy API description document ...';
-    const message = '... dummy error message ...';
-    let compilationResult;
-
-    beforeEach((done) => {
-      const stubbedDreddTransactions = proxyquire('../../lib/index', {
-        './parse': (input, callback) => callback(new Error(message), { dummy: true }),
-      });
-      stubbedDreddTransactions.compile(apiDescription, null, (err, result) => {
-        compilationResult = result;
-        done(err);
-      });
-    });
-
-    it('produces one annotation, no transactions', () => assert.jsonSchema(compilationResult, createCompilationResultSchema({
-      annotations: 1,
-      transactions: 0,
-    })));
-
-    it('turns the parser error into a valid annotation', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
-      type: 'error',
-      message,
-    })));
-  });
-
-  describe('when parser unexpectedly provides malformed API Elements only', () => {
-    const apiDescription = '... dummy API description document ...';
+  describe('when parser unexpectedly provides an error', () => {
+    const error = new Error('... dummy message ...');
+    let err;
     let compilationResult;
 
     beforeEach((done) => {
       const stubbedDreddTransactions = proxyquire('../../lib/index', {
-        './parse': (input, callback) => callback(null, { dummy: true }),
+        './parse': (apiDescription, callback) => callback(error),
       });
-      stubbedDreddTransactions.compile(apiDescription, null, (err, result) => {
-        compilationResult = result;
-        done(err);
+      stubbedDreddTransactions.compile('... dummy API description document ...', null, (...args) => {
+        [err, compilationResult] = args;
+        done();
       });
     });
 
-    it('produces one annotation, no transactions', () => assert.jsonSchema(compilationResult, createCompilationResultSchema({
-      annotations: 1,
-      transactions: 0,
-    })));
-
-    it('produces an error about parser failure', () => assert.jsonSchema(compilationResult.annotations[0], createAnnotationSchema({
-      type: 'error',
-      message: 'parser was unable to provide a valid parse result',
-    })));
+    it('passes the error to callback', () => assert.equal(err, error));
+    it('passes no compilation result to callback', () => assert.isUndefined(compilationResult));
   });
 });

test/unit/parse-test.js

@@ -1,5 +1,4 @@
 const fury = require('fury');
-const sinon = require('sinon');
 
 const parse = require('../../lib/parse');
 
@@ -44,7 +43,7 @@ describe('parse()', () => {
       done();
     }));
 
-    it('produces error', () => assert.instanceOf(error, Error));
+    it('produces no error', () => assert.isNull(error));
     it('produces API Elements', () => assert.isObject(apiElements));
     it('produces media type', () => assert.match(mediaType, reMediaType));
     it('the parse result contains annotation elements', () => assert.isFalse(apiElements.annotations ? apiElements.annotations.isEmpty : undefined));
@@ -69,25 +68,6 @@ describe('parse()', () => {
     it('the annotations are warnings', () => assert.equal(apiElements.warnings ? apiElements.warnings.length : undefined, apiElements.annotations.length));
   }));
 
-  describe('when unexpected parser behavior causes \'unexpected parser error\'', () => {
-    let error;
-    let apiElements;
-
-    beforeEach((done) => {
-      sinon.stub(fury, 'parse').callsFake((...args) => args.pop()());
-      return parse('... dummy API description document ...', (err, parseResult) => {
-        error = err;
-        if (parseResult) { ({ apiElements } = parseResult); }
-        return done();
-      });
-    });
-    afterEach(() => fury.parse.restore());
-
-    it('produces error', () => assert.instanceOf(error, Error));
-    it('the error is the \'unexpected parser error\' error', () => assert.include(error.message.toLowerCase(), 'unexpected parser error'));
-    it('produces no parse result', () => assert.isNull(apiElements));
-  });
-
   describe('when completely unknown document format is treated as API Blueprint', () => {
     let error;
     let mediaType;
@@ -104,7 +84,7 @@ describe('parse()', () => {
     it('produces media type', () => assert.match(mediaType, reMediaType));
     it('the parse result contains annotation elements', () => assert.isFalse(apiElements.annotations ? apiElements.annotations.isEmpty : undefined));
     it('the annotations are warnings', () => assert.equal(apiElements.warnings ? apiElements.warnings.length : undefined, apiElements.annotations.length));
-    it('the first warning is about falling back to API Blueprint', () => assert.include(apiElements.warnings.getValue(0), 'to API Blueprint'));
+    it('the first warning is about falling back to API Blueprint', () => assert.include(apiElements.warnings.getValue(0), 'assuming API Blueprint'));
   });
 
   describe('when unrecognizable API Blueprint is treated as API Blueprint', () => {
@@ -123,6 +103,6 @@ describe('parse()', () => {
     it('produces media type', () => assert.match(mediaType, reMediaType));
     it('the parse result contains annotation elements', () => assert.isFalse(apiElements.annotations.isEmpty));
     it('the annotations are warnings', () => assert.equal(apiElements.warnings.length, apiElements.annotations.length));
-    it('the first warning is about falling back to API Blueprint', () => assert.include(apiElements.warnings.getValue(0), 'to API Blueprint'));
+    it('the first warning is about falling back to API Blueprint', () => assert.include(apiElements.warnings.getValue(0), 'assuming API Blueprint'));
   });
 });