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

fix(src/plugins/utils/findOctetSequencePaths.js): Show path to erring… #190

Merged
merged 10 commits into from
Oct 23, 2020

Conversation

distortedsignal
Copy link
Contributor

@distortedsignal distortedsignal commented Aug 27, 2020

… element on exception

Recently, I ran across an issue where if the validator throws an
exception in a specific part of the code, the path to the
exception-causing code is swallowed
.

After a moderate amount of digging, I decided that an
option for providing more information about this code
would be to put the path to the problematic element or
elements into the exception message. This exception
message is displayed whenever the code crashes out. So,
for example, the previous error message output by
lint-openapi was the following:

Validation Results for epic_tenant.json:

[Error] There was a problem with a validator.
Cannot read property 'type' of null

TypeError: Cannot read property 'type' of null
    at arrayOctetSequences (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:39:20)
    at findOctetSequencePaths (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:22:34)
    at /usr/local/lib/node_modules/ibm-openapi-validator/
                             src/plugins/utils/
                             findOctetSequencePaths.js:72:14
    at Array.forEach (<anonymous>)
    at objectOctetSequences (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:58:35)
    at findOctetSequencePaths (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:24:34)
    at /usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:72:14
    at Array.forEach (<anonymous>)
    at objectOctetSequences (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:58:35)
    at findOctetSequencePaths (/usr/local/lib/node_modules/
                             ibm-openapi-validator/src/
                             plugins/utils/
                             findOctetSequencePaths.js:24:34)

After this change, the output should be the following:

[Error] There was a problem with a validator.
items.type and items.format must resolve for the path "paths/my/datatype/path/etc/etc"

TypeError: Cannot read property 'type' of null
    at arrayOctetSequences (/usr/local/lib/node_modules/
                            ibm-openapi-validator/src/
                            plugins/utils/
                            findOctetSequencePaths.js:39:22)
    (same stack as above, I would rather not edit it again)

re #180

@distortedsignal
Copy link
Contributor Author

Sorry about how the title on this PR got chowdered, but I tried to use git cz and that's what came out. I can change it if you (pl. "the repo-maintainers") would like.

@distortedsignal
Copy link
Contributor Author

Hey @mkistler - the PR that I referenced in #180 should be here. Looking forward to your comments.

… element on exception

Recently, I ran across an issue where [if the validator throws an exception in a specific part of
the code, the path to the exception-causing code is
swallowed](IBM#180). After a moderate amount of digging,
I decided that an option for providing more information about this code would be to put the path to
the problematic element or elements into the exception message. This exception message is displayed
whenever the code crashes out. So, for example, the previous error message output by lint-openapi
would say "Cannot read property 'type' of null", whereas the new error would list the full path to
the type that was causing the exception.

re IBM#180
@distortedsignal
Copy link
Contributor Author

Hey @dpopp07 @jorge-ibm and @mkistler - I know this probably isn't high on your priority list, but I do think this code would help out the community.

Copy link
Contributor

@jorge-ibm jorge-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So all it looks like you're doing is enhancing the error handling in this scenario, which I think is fine. I'll let @mkistler @dpopp07 give their opinion on this, since they might have some other thoughts.

Thanks for the contribution!

Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the detailed tests for that module as well

@dpopp07 dpopp07 merged commit 0247888 into IBM:master Oct 23, 2020
@distortedsignal distortedsignal deleted the show-escaped-path-on-exception branch October 23, 2020 16:42
@distortedsignal
Copy link
Contributor Author

I figured being a good citizen meant bumping up the test coverage as well. Hope this project goes great in the future! I might have one more PR for the infinite recursion issue (#179).

dpopp07 pushed a commit that referenced this pull request Oct 23, 2020
## [0.31.1](v0.31.0...v0.31.1) (2020-10-23)

### Bug Fixes

* **findOctetSequencePaths:** Show path to erring element ([#190](#190)) ([0247888](0247888)), closes [#180](#180)
@dpopp07
Copy link
Member

dpopp07 commented Oct 23, 2020

🎉 This PR is included in version 0.31.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants