parser-js: Improve error handling in parser

Reason/Context

On the Slack, there was a discussion in which several proposals were proposed to improve the handling of errors in parser.

Description

Proposals:

Add custom-validation-error new error type for custom validation like: validate variables of server name etc…
Create the predefinied classes for error types like in models:

class ValidationError extends ParserError {
  super(...args) {
     args.type = "validation-errors";
     super(args);
  }

  // additional logic
}

Think about subtypes for custom-validation-errors, for channels, server-variables etc. It would be great if it will be implemented with HTTP Problems RFC.

NOTE: this is improvement not for 1.0.0 but for the next version.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 19 (9 by maintainers)

Most upvoted comments

In Parser v2 we have improved error handling, but we still need that library https://github.com/asyncapi/problem to use.

magicmatatjahu on Dec 20, 2022

@derberg @BOLT04

About --fix flag, as Łukasz wrote, there is no error to be easily fixed, e.g. in eslint --fix is only to fix some problems with indentation, white spaces etc, and in AsyncAPI doc errors are related to the wrong structure so --fix won’t help, because we can’t change the content of the spec if something is wrong.

Maybe we should just remove the validation I added some time ago and it should belong to a separate “linter” solution. Maybe validation in parser should be simple, error only and strict-spec-only

I had the same thought. I wanna test Spectral from the Stoplight but I don’t have time. I wonder if we could reuse the logic related with rulesets and integrate them into our parser, then all the custom logic associated with validating duplicate tags, existing servers in channels etc (things that we cannot validate by JSON Schema) could be implemented using appropriate rules in the linter. Additionally, users would be able to add their own rules.

magicmatatjahu on Dec 20, 2021

I have to admit this one is pretty old and I don’t remember much 😅

I think taking inspiration on the HTTP Problems RFC is a good idea, but I’m not sure parser-js should return errors with all the fields of that spec, mainly the status field. This is because it would add a “dependency” on HTTP, and the parser can be used in another context, so HTTP status codes don’t mean a lot to the CLI, IMO.

I don’t think status makes a lot of sense, but I also think the intention here is to follow HTTP Problems RFC to the extend it makes sense.

Kinda like eslint --fix

The flag could be easily added to the validate command, but the question is if there is actually anything that could be auto-fixed really. I don’t think it is possible. At least there is no error that comes to my mind on Monday morning that could be fixed with some autofix.

what do you think about having an object, and inside it have that array of error types to ignore

Yup, object it is. My only real concern here is where to draw a line between a spec-related validation and a linter. I’m just not 100% sure if we should go in this direction at all. Maybe we should just remove the validation I added some time ago and it should belong to a separate “linter” solution. Maybe validation in parser should be simple, error only and strict-spec-only. Do you know what I mean? I’m kinda afraid that with one silent error we open a door for more and more, etc.

derberg on Dec 20, 2021