apiaryio / fury-adapter-swagger

Swagger 2.0 parser adapter for Fury.js
MIT License
11 stars 12 forks source link

Prevent causing two errors while handling a YAML error #138

Closed kylef closed 7 years ago

kylef commented 7 years ago

Prevent throwing or attaching additional warning while handling a source YAML document that produces an error while using the generateSourceMap flag.

There was a race condition where the done callback can be called twice as we would call the callback with an error and fury would catch a raised error and then return that error.

   1) Swagger 2.0 adapter cannot parse invalid Swagger YAML returns error for bad input yaml with source maps

       done() called multiple times

       at Suite.<anonymous> (test/adapter.js:125:5)

       125 |     it('returns error for bad input yaml with source maps', (done) => {
       126 |       fury.parse({ source, generateSourceMap: true }, (err, parseResult) => {

       at Suite.<anonymous> (test/adapter.js:112:11)

       112 |   context.only('cannot parse invalid Swagger YAML', () => {
       113 |     const source = 'swagger: "2.0"\nbad: }';

This would also cause a state when parsing an invalid YAML document would produce an error AND a warning in the returned parse result.

The "hot fix" here is disabling generateSourceMap for the duration of handling the YAML error so that we do not attempt to parse the YAML document again to produce source maps. Ideally we would have a single YAML parsing state so we wouldn't need this work around.

This problem causes dredd-transactions to fail.

kylef commented 7 years ago

NOTE there is a lot going on here, so if a reviewer needs any help feel free to reach out in person and I can walk through the change.