fix: prevent infinite recursion for circular tuple array schemas#2438
fix: prevent infinite recursion for circular tuple array schemas#2438Varadraj75 wants to merge 2 commits intoasyncapi:masterfrom
Conversation
✅ Deploy Preview for modelina canceled.
|
There was a problem hiding this comment.
Pull request overview
Fixes a stack overflow caused by infinite recursion when converting circular tuple-based array schemas in AsyncAPIInputProcessor.convertToInternalSchema() by properly propagating the recursion guard map during tuple item conversion.
Changes:
- Propagate
alreadyIteratedSchemasintoconvertToInternalSchema()calls for tuple-arrayitemsduring.map(). - Prevent infinite recursion for circular schemas embedded inside tuple-array items (fixes #2436).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull Request Test Coverage Report for Build 21875539290Details
💛 - Coveralls |
|
Hi @jonaslagoni , All checks are passing now and the fix is ready for review. Whenever you get a moment, could you please take a look? |
jonaslagoni
left a comment
jonaslagoni
left a comment
There was a problem hiding this comment.
Always make sure to add tests that show the error you found and that now passes 🙂
Varadraj75
force-pushed
the
fix/circular-tuple-array-recursion
branch
from
0c12688 to
134a3f9
Compare
Varadraj75
requested review from
asyncapi-bot-eve,
kennethaasan and
magicmatatjahu
as code owners
|
@jonaslagoni , |
| if (typeof schema === 'object') { | ||
| if (visitedSchemas.has(schema)) { | ||
| return {}; | ||
| } | ||
| visitedSchemas.add(schema); | ||
| } | ||
|
|
There was a problem hiding this comment.
Any reason you duplicating this? https://github.com/Varadraj75/modelina/blob/88b0ffcb0667ca2cae0f52c46532434ca1b383db/src/processors/AsyncAPIInputProcessor.ts#L224
There was a problem hiding this comment.
@jonaslagoni, thanks for pointing this out
Good catch — this was duplicated unintentionally while wiring the recursion guard into tuple item conversion. The logic already exists later in the function, so re-adding it here isn’t necessary.
I’ve now refactored it to reuse the existing recursion-guard handling instead of duplicating it, keeping the behavior unchanged while avoiding redundancy.
Please let me know if this looks good now or if you’d prefer a different structure here. Thanks!
Varadraj75
force-pushed
the
fix/circular-tuple-array-recursion
branch
2 times, most recently
from
ee5ea95 to
6d8cba8
Compare
Varadraj75
force-pushed
the
fix/circular-tuple-array-recursion
branch
from
6d8cba8 to
bd1494e
Compare
|
jonaslagoni
left a comment
jonaslagoni
left a comment
There was a problem hiding this comment.
Anything from this code that does not make sense?
|
@jonaslagoni thanks for the follow-up I’ve gone through the updated code once more, especially around the recursion guard handling and tuple item conversion, and everything looks consistent to me now. The duplicated guard logic has been removed, the existing guard is reused correctly, and the added test reproduces the original issue and passes with the fix in place. I don’t see anything remaining that doesn’t make sense or could be simplified further without changing behavior. Happy to adjust if you spot anything specific or would like a different structure — thanks again for the detailed review. |
3 participants
Description
Fixes infinite recursion when processing circular tuple-based array schemas
by correctly propagating the recursion guard during tuple item conversion.
Includes a minimal, safe change with no behavioral impact on non-circular schemas.
Related Issue
Fixes #2436
Checklist
npm run lint).npm run test).Additional Notes
NONE