fix(openapi): memory leak in large file handling within oas-normalize
#784
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erunion
added
bug
erunion
changed the title
oas-normalize
erunion
requested review from
kanadgupta,
a team,
Dashron and
domharrington
and removed request for
a team
kanadgupta
approved these changes
Mar 27, 2023
kanadgupta
pushed a commit
that referenced
this pull request
Mar 27, 2023
# [8.6.0-next.17](v8.6.0-next.16...v8.6.0-next.17) (2023-03-27) ### Bug Fixes * memory leak in large file handling within openapi-parser ([#784](#784)) ([1b1cc00](1b1cc00)) [skip ci]
kanadgupta
pushed a commit
that referenced
this pull request
Mar 29, 2023
# [8.6.0](v8.5.0...v8.6.0) (2023-03-29) ### Bug Fixes * bad merge ([e15c574](e15c574)) * bump node version in release workflow ([7f3158f](7f3158f)) * does this work? ([c81e432](c81e432)) * memory leak in large file handling within openapi-parser ([#784](#784)) ([1b1cc00](1b1cc00)) * next channel ([ce4e494](ce4e494)) * **openapi:** yaml strings would be improperly parsed as Date objects ([#779](#779)) ([72e75cb](72e75cb)) * rebuild prior to npm publish ([29b9ec6](29b9ec6)) * reformat github release header ([38c5625](38c5625)) * reformat header again ([bd2e1a2](bd2e1a2)) * remove some of the package scripts ([3eb52fd](3eb52fd)) * remove unnecessary config ([c22889c](c22889c)) * run tests but NOT release workflow on release commits ([24f885e](24f885e)) * temporarily disable release workflow ([a935268](a935268)) * try rearranging steps like this ([cac0c1d](cac0c1d)) * try this approach to lifecycle events ([4e5ecff](4e5ecff)) * try this as an alternative to @semantic-release/github ([8c343a0](8c343a0)) * try this to see if branch protections work ([f314c3f](f314c3f)) * turns out these rules weren't redundant ([f9f82f1](f9f82f1)) * upgrading `oas-normalize` to move to our `postman-to-openapi` fork ([#776](#776)) ([ee8ce0a](ee8ce0a)) ### Features * add git + changelog plugins ([85e4bfd](85e4bfd)) * container registry ([#777](#777)) ([d193416](d193416)), closes [/github.com//pull/777#discussion_r1145516673](https://github.com//github.com/readmeio/rdme/pull/777/issues/discussion_r1145516673) [/github.com//pull/777#discussion_r1145528308](https://github.com//github.com/readmeio/rdme/pull/777/issues/discussion_r1145528308) * docker (again) ([#763](#763)) ([2144572](2144572)), closes [#746](#746) * empty commit to trigger release ([3e3c112](3e3c112)) * fix comment ([076cfbf](076cfbf)) ### Performance Improvements * **docker:** build executable ([#764](#764)) ([af2dbce](af2dbce)) ### Reverts * Revert "feat: drop duplicative tag" ([f9fe6c6](f9fe6c6)) * bring workflow name back ([c07495a](c07495a)) * don't set header for changelog ([194489e](194489e)) * restore release workflow ([9f6bbc9](9f6bbc9)) * ugh here we go again ([0b1e429](0b1e429)) [skip ci]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🧰 Changes
This bumps oas-normalize, @readme/openapi-parser, and @readme/better-ajv-errors in order to resolve a pretty serious issue with large OpenAPI definitions causing memory leak issues within validation.
For some context,
@readme/better-ajv-errorsuses @babel/code-frame to generate error messages like the following:The problem we're having though is that
@babel/code-framewas running code highlighting on supplied OpenAPI JSON definitions for each error we were processing. In the case that spawned this discovery, the Cloudflare API schema, is upwards of 4MB formatted and contains well over 1,000 errors.@babel/code-framewas highlighting this 4MB file over 1,000 times.To resolve this I've implemented a couple fixes:
@readme/better-ajv-errors. As it was always highlighting our JSON as green It added nothing of value to me at the expense it cost.1@readme/openapi-parser. Even with the removal of code highlighting still generating code frames for 1,000+ errors was a massively expensive operation so I've modified@readme/openapi-parserto cap the amount of errors we show at 20 if the file is over a certain size. When this happens our validation error messaging will also now tell the user that they have an additional amount of errors that they will see when they re-run validation after fixing the errors we've shown them.2🧬 QA & Testing
Without the above fixes running
openapi:validateon this Cloudflare API definition took 15 minutes because I gave up and killed the process.With these dependency updates re-running
openapi:validateon the same API definition now only surfaced the first 20 errors, took 13 seconds, and told the user that they have an additional 1016 errors.After fixing the most common error in this API definition,
4xxresponse HTTP codes needing to be4XX, validation now returned 2 errors, finished in 7 seconds, and didn't tell the user they any additional errors.Like I mentioned in readmeio/openapi-parser#180 I don't love that we're masking errors for these situations but we don't have any control over
@babel/code-frame, there aren't any better alternatives for it out there, and I have no intention of trying to figure out ways to hack or fork a better solution into it.Footnotes
https://github.com/readmeio/openapi-parser/pull/180 ↩
https://github.com/readmeio/openapi-parser/pull/180 ↩