-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Maximum call stack size exceeded of complicated schemas (FHIR for example) #4411
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
Comments
Yep, that is one XXL complex definition... |
@igrishaev I noticed that you do have one file that seems to bundle all the definitions, encountered a few issues one of them was: After making the corrections to the definition the UI loads fast: |
After playing with the 4411_swagger.json I noticed that expanding the models is a bit sluggish but it works ... but expanding the GET completely crashes the browser |
@heldersepu yes, I managed to compose my own big schema out from those files. I excluded some of them. |
@igrishaev On that link I sent when I click on GET the memory starts climbing up until it crashes... |
@heldersepu sorry I cannot reproduce that at the moment, but I guess the result would be the same. In Chrome, the tab hangs and stops responding to any action, and my laptop becomes hot. |
Hmm... this is either (a) making the resolver recursively loop or (b) legitimately filling up the call stack, which is 10402 frames in Chrome. @heldersepu, we've discussed the shallow resolver concept ( |
@shockey Yes we talked about that on the Lazy loading of the models... Do you know what component is causing this crash? |
@heldersepu, I'm still led to believe it's the resolver: when the operation is expanded, everything within the operation gets resolved, all the way down, until there are no $refs left. |
I just did a quick search for Now this is not to cover for Swagger-UI, there are issues with large/complex schemas. |
@shockey almost afraid to ask... but what exactly does happen if the same ref appears 1000 times within the same schema? |
@webron Here is a test schema with 1000 refs. (spoiler alert: it expands in milliseconds) http://petstore.swagger.io/?url=http://swagger-net-test.azurewebsites.net/api/BigSwag/1000 That schema has one model (Test1K) with 1000 properties all are ref to the Location model. Update:I tested with as many as 10K refs using the same flat structure and it get slower but no crash |
@shockey & @webron Sorry I like breaking things! Here is a very small (~3KB) schema, only a few models (but they are nested) Looks to me we have a Big O(n!) lurking somewhere... |
@heldersepu I agree, looks like it has more to do with depth than size. |
@shockey so back to your shallow resolver concept ( |
We have been also hit by this issue in version 2.2.10.
|
@prochnowc that's separate as it's a completely different code base. We also no longer provide support for 2.X. |
I've been questioning why do we need to fully expanded a model? So here is my question @shockey with a shallow resolver:
|
Here are some scary numbers
I was afraid to do it with a 6, Copy/Pasting the HTML was taking and awful long time |
Maybe for extremely large object (models, examples, responses, etc) we should do the same thing that GitHub does, show the user a message and call that the object is too big:
https://github.com/swagger-api/swagger-ui/blob/master/dist/swagger-ui-bundle.js |
you'd still be able to go arbitrarily deep: the resolver takes its last output as an input to avoid redoing work, so if you were resolving 1 level at a time, you'd go one level deeper each time you called it.
We do have to iterate over each occurrence of the $ref to replace it, but we do avoid needlessly iterating over the results of those operations (thanks to swagger-api/swagger-js#1209). |
@shockey I'm afraid that without a "hard depth max expansion" the UI will always have this issue ... The expansion of a truly complex object creates a LOT of code, the browser will ultimately crash and burn, that's what I was researching on my scary numbers comment, looks like an exponential increase in size. |
@shockey about the "avoid needlessly iterating" I think we can still do better ... Take a look at the nested 7 example: The Model |
This can't be really overstated: with your examples of 5 fields in each level of nesting, you end up with
Definitely. Since $ref resolution is depth-first, we can go as far as caching each $ref as we walk down the tree. That would take a lot of work off of the resolver, since it would only need to reach for each unique $ref's value once, after which it would just grab the already-computed data out of the cache. -- As for this ticket, I'm going to take this out of my queue since the stack overflow is more related to performance than a flaw in the resolver. Let's keep the discussion of solutions going though 😄 |
A note from #4637 that was closed as duplicate of this issue, following is the specification causing this issue for me:
|
How I reproduce this:
After failing to load several refs, the I set a break point here and then traced back to here. And I found the reducer which throws the error. I saved the action payload to a global variable and tried to stringify it in order to view in a JSON viewer and it turned out to be a circular structure. With further digging I found the cause: the {
"$schema": "http://json-schema.org/draft-04/schema#",
"id": "http://hl7.org/fhir/json-schema/Element",
"$ref": "#/definitions/Element",
"description": "see http://hl7.org/fhir/json.html#schema for information about the FHIR Json Schemas",
"definitions": {
"Element": {
"allOf": [
{
"description": "Base definition for all elements in a resource.",
"properties": {
"id": {
"description": "unique id for the element within a resource (for internal references). This may be any string value that does not contain spaces.",
"type": "string"
},
"_id": {
"description": "Extensions for id",
"$ref": "Element.schema.json#/definitions/Element"
},
"extension": {
"description": "May be used to represent additional information that is not part of the basic definition of the element. In order to make the use of extensions safe and manageable, there is a strict set of governance applied to the definition and use of extensions. Though any implementer is allowed to define an extension, there is a set of requirements that SHALL be met as part of the definition of the extension.",
"type": "array",
"items": {
"$ref": "Extension.schema.json#/definitions/Extension"
}
}
}
}
]
}
}
} There's nothing wrong with the model definition, but for now swagger UI cannot handle it. Turning the cyclic json definition into an immutable map is an issue; while generating an example to view and edit is another. @shockey |
@bestmike007 thanks for the detailed report... |
It is the original demonstration API definition provided by @igrishaev |
@bestmike007 Ohh that one is huge... |
Yes. @heldersepu {
"swagger": "2.0",
"schemes": ["http"],
"info": {
"contact": {
"email": "[email protected]"
},
"title": "sdfsdfsf"
},
"paths": {
"/foo": {
"get": {
"description": "sdfsdfsdfdsf",
"responses": {
"200": {
"description": "sdfsdf",
"schema": {
"$ref": "#definitions/Element"
}
}
}
}
}
},
"definitions": {
"Element": {
"allOf": [
{
"description": "Base definition for all elements in a resource.",
"properties": {
"id": {
"description": "unique id for the element within a resource (for internal references). This may be any string value that does not contain spaces.",
"type": "string"
},
"_id": {
"description": "Extensions for id",
"$ref": "Element.schema.json#/definitions/Element"
}
}
}
]
}
}
} |
@bestmike007 I think that here: |
Sorry for that. It seems that it has to be referring an external file to reproduce the bug: 3.json {
"swagger": "2.0",
"paths": {
"/foo": {
"get": {
"responses": {
"200": {
"schema": {
"$ref": "#definitions/Element"
}
}
}
}
}
},
"definitions": {
"Element": {
"allOf": [
{ "$ref": "4.json#/definitions/Extension" }
]
}
}
} 4.json {
"definitions": {
"Extension": {
"allOf": [
{
"$ref": "3.json#/definitions/Element"
}
]
}
}
} You might need to kill the tab using the task manager: http://127.0.0.1:5000/swagger/?url=/3.json#/default/get_foo |
Strange that it does not happen if all the definitions are in one file: |
So that does not have anything to do with my issue in the comment: #4411 (comment) And the schema reproducing the issue is not large or complicated... |
Latest version seems to have fixed all the issues I encountered before:
@shockey you can close this issue |
thanks as always for making my job easier, @heldersepu! |
On large schemas, Swagger UI either fails with
RangeError: Maximum call stack size exceeded
error or hangs with 100% CPU usage. In my case, these were the official FHIR schemas which may be downloaded by a link: https://www.hl7.org/fhir/fhir.schema.json.zip It would be enough to unzip them and copy thefhir.schema.json
folder next to your swagger spec.Demonstration API definition
Configuration (browser query string, constructor, config.yaml)
I didn't change enything except the
url
parameter:url: "/swagger/schema",
Expected Behavior
To see the schemas.
Current Behavior
Either an exception (3.13.0) or an endless 100% CPU usage (3.13.2)
Context
I figured out by crafting a small subset of definitions out of these schemas.
The text was updated successfully, but these errors were encountered: