raml-org / raml-js-parser-2

(deprecated)
Other
138 stars 53 forks source link

Parser Async should reject the Promise when parsing errors are found #35

Closed juancoen closed 8 years ago

juancoen commented 8 years ago

Right now, the promise is fulfilled, returning an invalid API representation and clients must call the errors() method to know if there was an error.

KonstantinSviridov commented 8 years ago

Some users may need even that API representation, which contains errors. That's why we do not reject Promise in this situation.

Is it important for you to have Promise rejected in case of API with errors? If yes, we can introduce a special option for this purpose.

sichvoge commented 8 years ago

Would that change the high-level Parser API?

KonstantinSviridov commented 8 years ago

@sichvoge A little bit. There will be a new field in http://raml-org.github.io/raml-js-parser-2/interfaces/_raml1_wrapped_ast_parsercore_.options.html

juancoen commented 8 years ago

ok, I agree with having an option for rejecting promises on errors, to cover all possible scenarios

blakeembrey commented 8 years ago

What will the rejected promise look like? Rejections should be error instances, but the parser can return multiple errors. An error will errors on a property, or just the first error instance?

KonstantinSviridov commented 8 years ago

@blakeembrey For now it's just an error containing a single message. Do you know some Error subclass which can be passed an object (or array) to constructor (just like we pass strings usually)?

blakeembrey commented 8 years ago

There's nothing formal in JavaScript, but you can just do it yourself. You can also subclass Error if you want to do this in a declarative way (with something like https://github.com/julien-f/js-make-error).

var error = new Error('Invalid RAML')
error.errors = errors // Tada, just document the error type.
return error
KonstantinSviridov commented 8 years ago

Ok, thanks. So, there will be a message Api contains errors. and a complete array of errors.

KonstantinSviridov commented 8 years ago

@blakeembrey It appears that Error subclassing is not available in typescript 1.4 https://github.com/Microsoft/TypeScript/issues/1168.

Seems like it remains to write parser errors to some field of the newly created error.

blakeembrey commented 8 years ago

@KonstantinSviridov I pointed out how you can do it in the comment above.

KonstantinSviridov commented 8 years ago

For loadApi: If the 'rejectOnErrors' option is set to true, ApiLoadingError is thrown for Api which contains errors.

For loadApiAsync: The Promise is rejected with ApiLoadingError if the resulting Api contains errors and the 'rejectOnErrors' option is set to 'true'.

Where ApiLoadingError is http://raml-org.github.io/raml-js-parser-2/interfaces/_raml1_wrapped_ast_parsercore_.apiloadingerror.html