Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing other yaml files leads to problem report without a source #51

Closed
eseglem opened this issue Aug 2, 2023 · 7 comments
Closed

Comments

@eseglem
Copy link

eseglem commented Aug 2, 2023

What happened?

The extension tries to read all yaml files in the workspace and not all of them are OpenAPI. Any yaml with custom tags seem to confuse the extension. Rather than reporting nicely, it ends up with a problem without a "source".

[{
	"resource": "/path/to/example.yaml",
	"owner": "_generated_diagnostic_collection_name_#0",
	"severity": 8,
	"message": "unknown tag !<!GetAtt> in \"/path/to/example.yaml\" (12:13)",
	"startLineNumber": 12,
	"startColumn": 13,
	"endLineNumber": 12,
	"endColumn": 13
}]

What should have happened instead?

  • Handle reading non OpenAPI yaml with custom tags and report the issue with a "source" included so it is easier to track down.
  • It would be nice to allow explicit include / exclude of files / directories that the extension will check. Then it does not need to deal with files which aren't OpenAPI.

Minimal reproducible OpenAPI snippet

CloudFormation from: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/gettingstarted.templatebasics.html

Resources:
  myBucket:
    Type: 'AWS::S3::Bucket'
  myDistribution:
    Type: 'AWS::CloudFront::Distribution'
    Properties:
      DistributionConfig:
        Origins:
          - DomainName: !GetAtt 
              - myBucket
              - DomainName
            Id: myS3Origin
            S3OriginConfig: {}
        Enabled: 'true'
        DefaultCacheBehavior:
          TargetOriginId: myS3Origin
          ForwardedValues:
            QueryString: 'false'
          ViewerProtocolPolicy: allow-all

Additional context

This made it quite difficult to track down where the reported problem was coming from. It required disabling extensions until the problem disappeared to figure out it was Redocly.

@RomanHotsiy
Copy link
Member

Thanks for the report @eseglem 🙌

@tatomyr @roman-sainchuk just making sure you see this ☝️

@tatomyr
Copy link
Contributor

tatomyr commented Aug 7, 2023

Hi @eseglem,

What exactly do you mean by custom tags?
Also, could you check if those yaml files are somehow referenced from an actual openapi definition file?

@eseglem
Copy link
Author

eseglem commented Aug 9, 2023

@tatomyr Custom tags being any local tags that aren't specifically part of the yaml spec. They are left up to the application to handle. Pretty much anything with an ! at the beginning. In the above example this is !GetAtt but the specific tag doesn't matter they all cause the same issue.

In my case they are completely independent and unrelated to openapi. You don't even need to have an openapi definition file to see the problem show up.

Isolated steps to reproduce:

  • Open an empty folder in VSCode
  • Save the above as example.yaml
  • Allow extension to create a redocly.yaml for you.
  • You now have 1 problem in example.yaml

@roman-sainchuk
Copy link
Contributor

@eseglem Thanks for reporting! The issue is that extension cannot parse this yaml file so we cannot even determine if the file is the openapi spec or not.
We have to think about how to deal with such cases because we're not sure that showing no errors is acceptable.

@RomanHotsiy
Copy link
Member

@roman-sainchuk how about if we try to detect openapi from plaintext somehow and show errors only in such case?

@roman-sainchuk
Copy link
Contributor

@RomanHotsiy great idea! We'll try to implement it, thanks!

@eseglem
Copy link
Author

eseglem commented Aug 11, 2023

I don't think its a horrible thing to report an error, my main issue was that the error does not say it's coming from the Redocly extension. If you catch it and report the error with a source that would be enough to address that concern.

Though I would prefer to see a way to configure which files it tries to parse. Then I could tell it not to do those files. I may have many yaml files across many folders in a repo, but any openapi yaml are in one folder. There is no reason the extension needs to look at the other ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants