swagger-api / swagger-ui

Swagger UI is a collection of HTML, JavaScript, and CSS assets that dynamically generate beautiful documentation from a Swagger-compliant API.
https://swagger.io
Apache License 2.0
26.31k stars 8.91k forks source link

Large response bodies cause hanging #3832

Open shockey opened 6 years ago

shockey commented 6 years ago
Q A
Bug or feature request? Bug
Which Swagger/OpenAPI version? 2.0
Which Swagger-UI version? 3.4.0
How did you install Swagger-UI? local dev server
Which browser & version? Chrome 61
Which operating system? macOS High Sierra

Demonstration API definition

Swagger 2 Petstore is sufficient.

Configuration (browser query string, constructor, config.yaml)

Default.

Current Behavior

As an example, GET /pet/findByStatus in the Petstore currently returns 1.8MB of JSON when all statuses are selected. This is a pretty big chunk of data, but Swagger-UI insists on rendering the entire body, which is neither performant or useful. The main thread of the application is locked for upwards of 20 seconds on my machine.

Possible Solution

Truncate or refuse to display large textual response bodies.

Context

GET /pet/findByStatus is my favorite operation to use when testing UI enums, so this is something that impedes me pretty regularly.

shockey commented 6 years ago

cc @webron, would appreciate a priority assignment

heldersepu commented 6 years ago

This is strange! I swear I've used larger responses without any issues, let me see if I can make one up...

heldersepu commented 6 years ago

Here is my test: http://swashbuckletest.azurewebsites.net/swagger/ui/index?filter=Huge#/HugeResponse/HugeResponse_Get Yes we have a problem, even with 100000 it generates a 378K response that takes way too long

heldersepu commented 6 years ago

a quick fix could be not to display such a large responses, instead show something like github does on diff: Large diffs are not rendered by default.

or truncate the body to 20000:

<ResponseBody content={ body.substring(0,20000) }
        contentType={ contentType }
        url={ url }
        headers={ headers }
        getComponent={ getComponent }/>

If we truncate the response will show: can't parse JSON. Raw result:

heldersepu commented 6 years ago

Profile-20171028T214335.zip Attached is my performance profile I hope that can help troubleshooting. something in react is taking too long webpack:///./~/react-dom/lib/DOMChildrenOperations.js?568f

function removeChild(parentNode, childNode) {
  if (Array.isArray(childNode)) {
    var closingComment = childNode[1];
    childNode = childNode[0];
    removeDelimitedText(parentNode, childNode, closingComment);
    parentNode.removeChild(closingComment);
  }
  parentNode.removeChild(childNode);
}
webron commented 6 years ago

As @heldersepu suggested, I believe it's fair to limit the size that's being rendered, and if it's larger, make a link to download the result separately. This is not perfect though, as it means running the call again, and for some APIs it won't necessarily return the same result.

shockey commented 6 years ago

@webron, we'd be able to store the response string we already received, and allow the user to download it as a file - no second request needed.

As a proof of concept, see the Save as JSON/Save as YAML options in Swagger-Editor: we take the current editor content as a string and download it as a file, all from the client side.

webron commented 6 years ago

That's perfect then.

heldersepu commented 6 years ago

@shockey & @webron Love the solution! the download link will be awesome!

...but someone should try looking into why the slowdown You guys don't know any react guru that can truly get to the bottom of the issue?

webron commented 6 years ago

In the past, we had issues with the syntax highlighter and large responses, but that doesn't seem to be the issue. I've tested the same API call in both Nightly and Chrome, and they both render fairly quickly (~3 seconds) when the format is JSON. When the format is XML though... that's when it explodes.

shockey commented 6 years ago

Okay, I dug into this a bit more, and @webron's note about XML is spot on.

When rendering XML, React spends a ton of time in the ResponseBody render function:

messages image 4167817450

Everything else looks performant, so the good news is that our problem is contained to one component.

Doing a traditional profile on call stack times between clicking Execute and seeing the body revealed the true issue:

messages image 880229148

More than 95% of the main thread's time is spent parsing the whitespace-stripped XML that comes from the server into a pretty™ XML string. Even more specifically, this line appears to be the bottleneck: https://github.com/swagger-api/swagger-ui/blob/master/src/core/utils.js#L222

TL;DR: it's the formatXml(xmlStr) prettifier function in utils.js

heldersepu commented 6 years ago

I was looking at the formatXml function and that code smells funny... Inside that function there is a section where we:

It might be late and I'm missing something ... ...or that can be simplified, I'm sending you guys a PR

shockey commented 6 years ago

@heldersepu, I agree - note that the code was originally taken from an SO answer.

I'd be in favor of using a third-party module to handle the formatting. In the meantime, I'll take a look at your PR 😄

heldersepu commented 6 years ago

I think we still have a problem with large responses, my test: http://swashbuckletest.azurewebsites.net/swagger/ui/index?filter=Huge#/HugeResponse/HugeResponse_Get that is a json response and it gets uncomfortably slow to render...

I think we should still set a hard MAX for the response size and provide the download link for those large ones

shockey commented 6 years ago

@heldersepu, yeah, I see it with your example - the request comes back in under a second, but the rendering takes about ten seconds. I agree with the hard max, but I'd also like to continue looking at the underlying causes and try to address as we go.

Reopening.

Looks like the lion's share of that time (about 8 seconds) is React fiddling with the DOM. The response rendering causes 370(!!) DOM operations to be queued, and >90% of them appear to be caused by JumpToPath affecting ModelCollapse, which is unrelated to the response coming in as far as I can see.

This is a performance issue, but I'm inclined to keep this separated from our big performance ticket, since it's related to Try-It-Out, not initial rendering.

Below is a snapshot of React's DOM operation queue (the HighlightCode update with the response body isn't even on the first page):

masc3d commented 4 years ago

why not render asynchronously in stages, like eg. postman does. it works well and there's barely any need for switches or limits.

masc3d commented 4 years ago

btw this issue got much worse in recent versions, presumably due to pretty-print / color enhancements which make rendering even more expensive.

heldersepu commented 4 years ago

why not render asynchronously in stages, like eg. postman does. it works well and there's barely any need for switches or limits.

What's the purpose of showing/render a response larger than 2MB? That is certainly not humanly readable, my sample above on postman takes a few seconds too on the "formating" step... To me what github does on diff is the way to go, let the users know the response is large and give them an option to download content or render at own risk

masc3d commented 4 years ago

What's the purpose of showing/render a response larger than 2MB?

your argument makes no sense to me. if there's no purpose (in showing the content), why would you download it to eventually do exactly that.

download is okay, it would resolve the most important aspect of this issue. but it's still less convenient.

heldersepu commented 4 years ago

here are a couple of ideas why someone would download:

Hopefully, that makes the argument make sense to you

masc3d commented 4 years ago

if it makes sense to download it (very often) makes even more sense to view the content right in place.

just for quick checking a subset of a larger result, having to download, open in another editor and cleaning up afterwards are just an awful lot of unnecessary and tiring additional steps.

heldersepu commented 4 years ago

if it makes sense to download it (very often) makes even more sense to view the content right in place.

YES, and that is exactly what Swagger-UI is doing right now, it works perfectly for 99.9% of the cases ... but this issue is about those not very often cases, like those with large response bodies

There are purpose-built tools, Swagger-UI is no exception... From my days creating ETL jobs, I would not use a browser or a regular text editor to look at large data, instead, I get the schema definition, and load the data into a database for further processing/analysis.

This issue is more around bad practices on API design, responses should not be large, and if there is a need the architect should use pagination, that is why personally never attempted a code fix.

masc3d commented 4 years ago

This issue is more around bad practices on API design, responses should not be large,

highly depends on the use case.

anyway it's not implemented that way in postman for no reason, expensive operations should not block UI, talking about bad practices.

in latest builds it doesn't even take a really "large" result for swagger-ui to begin stalling.

heldersepu commented 4 years ago

anyway it's not implemented that way in postman for no reason, expensive operations should not block UI, talking about bad practices.

Remember the project is open source, if this is a big issue to you the code is there for you to fix it

masc3d commented 4 years ago

Remember the project is open source, if this is a big issue to you the code is there for you to fix it

it's not an issue at all for me. just some thoughts for a good solution, as it wasn't mentioned as an option before.

jmsteinbrunner commented 3 years ago

Configuration

I'd like to tag on to this issue. I've been using Swashbuckle.AspNetCore 5.5.1 (Swagger-UI v3.26.0 I believe) and have a somewhat large response body (20k lines of json give or take). In the 5.5.1, the request duration takes 45ms and displays almost instantly. Using the exact same configuration with 5.6.0 (Swagger-UI v.3.32.5), request duration takes 45ms and locks the interface 30 seconds and finally displays. Once that is rendered, if I minimize or expand anything else, the interface is locked again for 30 seconds each time. Once I clear the results, the interface speed returns to normal.

The coloring added is fantastic but I am currently stuck using the older version as the speed is simply too much of an issue.

Omzig commented 3 years ago

In order for the results to be displayed i have to .Take(200), if i let more than go through, it will not render and times out. When i revert to 5.5.1 the UI will render all the records almost instantly. I am now stuck with Swashbuckle.AspNetCore 5.5.1

lsanders commented 3 years ago

Ran into this recently too. Looks like you can turn it off via config. For me, this worked:

SwaggerUI({
        syntaxHighlight: {
          activated: false,
          theme: "agate"
        },
        //url: path,
        ....
      });
ahmed-abdelfata7 commented 3 years ago

I am using swagger with NodeJS and one of API response size is 1.54MB not working with me and also hanging

blekinge commented 3 years ago

I have at fix

Do something like this in your webservice method

//https://github.com/swagger-api/swagger-ui/blob/master/src/core/components/response-body.jsx#L61
//If you set the Content-Disposition to something containing attachment, it will not be rendered but made downloadable in swagger UI
//inline will be rendered directly in browser, if invoked not through swagger UI
httpServletResponse.setHeader("Content-Disposition", "inline; swaggerDownload=\"attachment\"; filename=\"filename.txt\"");

I.e. set the Content-Disposition header to something containg attachment

If you look at https://github.com/swagger-api/swagger-ui/blob/master/src/core/components/response-body.jsx#L61 you will see that swagger UI checks if the HTTP header "Content-Disposition" contains the string "attachment" If it does, the content is NOT shown directly, but hidden behind a download link. Note that response-body.jsx does NOT check anything about the well-formedness of the content disposition header. It just checks if it contains the word "attachment". For this reason I just dumped some extra field swaggerDownload="attachment";, in order to have the word "attachment" there, without confusing other systems.

If this API method is invoked normally from a browser, rather than from swagger UI, it would just render the content inline, as this is specified, and also the default behaivour if Content-Disposition is not specified

bovino commented 3 years ago

2021 here, using swagger-ui-express with node ( https://www.npmjs.com/package/swagger-ui-express ) latest version (4.1.6) and this problem is still there :-D

spyro2000 commented 3 years ago

Same here. Swagger still crashes from json larger than about 2 MB or so. There should be a size-limit for that where only pretty-printing is applied (no coloring).

Kraego commented 3 years ago

I have the same situation with asp .net 5 (the repsonse body is large, but far from huge 650kB json)

    <PackageReference Include="Swashbuckle.AspNetCore" Version="5.6.3" />
    <PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="5.6.3" />
    <PackageReference Include="Swashbuckle.AspNetCore.Newtonsoft" Version="5.6.3" />
    <PackageReference Include="Swashbuckle.AspNetCore.Swagger" Version="5.6.3" />
    <PackageReference Include="Swashbuckle.AspNetCore.SwaggerUI" Version="5.6.3" />
BjoernHund commented 2 years ago

We have the same problem. The swagger ui does not respond.

    <PackageReference Include="Swashbuckle.AspNetCore" Version="6.1.4" />
    <PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.1.4" />
Lex45x commented 2 years ago

@Kraego, @BjoernHund as mentioned above, you can speedup rendering by disabling syntax highlight. Here is example for AspNetCore.

 app.UseSwaggerUI(config =>
 {
    config.ConfigObject.AdditionalItems["syntaxHighlight"] = new Dictionary<string, object>
    {
        ["activated"] = false
    };
});
BjoernHund commented 2 years ago

@Kraego, @BjoernHund as mentioned above, you can speedup rendering by disabling syntax highlight. Here is example for AspNetCore.

 app.UseSwaggerUI(config =>
 {
    config.ConfigObject.AdditionalItems["syntaxHighlight"] = new Dictionary<string, object>
    {
        ["activated"] = false
    };
});

Yes, this solution works, but that's not the solution for the future.

Kraego commented 2 years ago

@Kraego, @BjoernHund as mentioned above, you can speedup rendering by disabling syntax highlight. Here is example for AspNetCore.

 app.UseSwaggerUI(config =>
 {
    config.ConfigObject.AdditionalItems["syntaxHighlight"] = new Dictionary<string, object>
    {
        ["activated"] = false
    };
});

Yes i can also confirm that it's a working workaround for the problem. @Lex45x c thank you.

spyro2000 commented 2 years ago
  1. Years. Later. Still the same problem and no flag available to disable pretty print... :(
lamaan commented 2 years ago

I may take a look at this, as the problem is an issue for me now. But for those who want to dig into this one, this will most likely be an issue with string concatenation in the body rendering. This usually involves the expensive process of allocating memory many times if done the wrong way (e.g. just using stringc = stringa + stringb repeatedly). I have seen this kind of issue several times in the past. Instead build a array of strings and then concatenate it.

Omzig commented 2 years ago

I may take a look at this, as the problem is an issue for me now. But for those who want to dig into this one, this will most likely be an issue with string concatenation in the body rendering. This usually involves the expensive process of allocating memory many times if done the wrong way (e.g. just using stringc = stringa + stringb repeatedly). I have seen this kind of issue several times in the past. Instead build a array of strings and then concatenate it.

Yep, it used to not be a problem prior to v5.5.1

jl398m commented 2 years ago

The issue doesn't seem to be caused by response size, but the number of records. I have a result with only 2 columns (int, date) but result sets >1000 records cause the UI to be awful, even though the total response is < 1mb. The same call with Postman renders in about a second.

Since swagger UI is only for demonstrating how to use the API, I'd like an easy way to set a max number of rendered records in swagger. Is that possible?

jl398m commented 2 years ago

And turning off syntax highlighting helped a lot, but I'd still rather not have swagger try to display so many records.

taranlu-houzz commented 2 years ago

I have recently hit this issue as well while working on an internal api using fastapi that can return json with many entries. I feel like the following combination of tweaks/options would be helpful:

Unfortunately, I can't offer to help with this right now, and I do get that changes like this would be a fair amount of work. Just wanted to add my two cents and note that this is an issue I have also run into.

masc3d commented 2 years ago
  • Unhighlighted first and run highlighting asynchronously

👍🏼 https://github.com/swagger-api/swagger-ui/issues/3832#issuecomment-674387896 high workloads do not belong on ui thread.

taranlu-houzz commented 2 years ago

@masc3d Yes, good point. It should either be a separate thread, or a subprocess that is run asynchronously.

sebastianhaberey commented 2 years ago

The workaround for those that use Spring (in application.yml):

springdoc:
  swagger-ui:
    syntax-highlight:
      activated: false
spyro2000 commented 2 years ago

Wish this could be automated if payload > x MB or something.

andiemmadavieswilcox commented 2 years ago

The disabling syntax highlighting workaround didn't work for me, so I kept exploring. I couldn't possibly think what was causing it to hang so badly, as my controller method and associated data structures weren't particularly chunky. Then I realised that I'm declaring in an attribute that my method will be returning a BadHttpRequestException in cases where I'm doing validation on the request data (this is in ASP.NET Core by the way):

Code screenshot

When seeing that the Swagger UI generates an example instance of this return type, this results in an enormous amount of JSON as Swagger populates every possible property recursively, which is what's causing the hanging in every single interaction in my case:

Generated JSON

Removing my usage of this attribute to declare this possible return type is what made the UI responsive for me again. This is obviously a workaround and not a solution, but I thought I'd share my findings here in case it helps other folk stop tearing their hair out!

Edit: It's just occurred to me that I shouldn't actually be throwing exceptions anyway, I should be returning BadRequestObjectResult implicitly by calling the BadRequest("My error message...") helper, and just stating that the action can return 400s with [ProducesResponseType((int)HttpStatusCode.BadRequest)] without stating a type! But the issue could still manifest for someone else if they declare an expected result type that has a potentially very deep tree structure.

LouiseYau commented 1 year ago

SwaggerUI({ syntaxHighlight: { activated: false, theme: "agate" }, //url: path, .... });

Is there anyway this workaround can be implemented in swagger-ui-react ? syntaxHighlight isn't an exposed prop

ad9999 commented 1 year ago

Any update / fix available for swagge-ui-react package ?