opendiffy / diffy

Other
1.25k stars 142 forks source link

Add support for maxResponseSize #45

Closed puneetkhanduri closed 1 year ago

puneetkhanduri commented 4 years ago

@sky-eduardolopes

sky-eduardolopes @sky-eduardolopes Jun 25 13:51 on another note, hoping someone can assist on this: in my tests I'm getting a response larger than the limit and causing a 500. I get " Throw(com.twitter.finagle.http.TooLongMessageException: Response entity too large" on the logs. Looking at a related issue I saw a suggestion to set "com.twitter.finagle.Http.param.MaxRequestSize" when starting the service. Since my error is due to response size I set two parameters when starting diffy, as such: "java -jar -Dcom.twitter.finagle.Http.param.MaxRequestSize=15728640.bytes -Dcom.twitter.finagle.Http.param.MaxResponseSize=15728640.bytes /root/Diffy/diffy-server.jar". Does anyone know how to make Diffy accept a larger response?

Puneet Khanduri @puneetkhanduri Jun 25 19:27 @sky-eduardolopes : thanks for your kind words. The flags you are already setting should be sufficient if your payload is less than 15 MB. Can you confirm the size of your payload?

sky-eduardolopes @sky-eduardolopes Jun 25 19:30 Content-Length: 6454768 Due to the lack of a responseMode I'm still using a quite old version of the openDiffy, before the isotope... Could this be the reason for those flags not applying?

sky-eduardolopes @sky-eduardolopes Jun 25 19:37 Due to the lack of a responseMode I'm still using a quite old version of the openDiffy, before the isotope...

actually one that I patched with theresponseMode commit from twitter/diffy

sky-eduardolopes @sky-eduardolopes Jun 25 20:32 FYI I had asked on the finagle gitter before asking here and this is what they told me: "those are Stack params which are not read from System properties. They’re generally set as code via the Finagle client"

puneetkhanduri commented 4 years ago

@sky-eduardolopes : Please verify that this works with the latest build. Please refer to the Quickstart for the relevant flags.

sky-eduardolopes commented 4 years ago

Hi @puneetkhanduri, sorry for the delay but I was away for a few days. From what I see the latest build doesn't have the latest commits (including the resposeMode commit), will you make a new one or should I just build locally?

sky-eduardolopes commented 4 years ago

I've done a local build and deployed it with the flags found on Qickstart.md. However I'm still getting a 500 on the same request. Unfortunately I can no longer see the request details on the Diffy service logs so I can't confirm it's the same error. How do I increase the logging level?

sky-eduardolopes commented 4 years ago

java -jar /root/Diffy/diffy-server.jar \ -candidate='my-candidate.com' \ -master.primary='my-primary.com' \ -master.secondary='my-secondary.com' \ -service.protocol='https' \ -serviceName=' Diffy Test' \ -maxHeaderSize='32.kilobytes' \ -maxResponseSize='10.megabytes' \ -proxy.port=:8882 \ -admin.port=:8883 \ -http.port=:8884 \ -rootUrl='localhost:8884' \ -excludeHttpHeadersComparison='true' \ -responseMode='candidate' \ -summary.email='me@email.com' \ -summary.delay='5' \ -allowHttpSideEffects='true' &\

HarshiShah commented 1 year ago

@puneetkhanduri Looks like the max header and response size options were removed as part of Spring boot migration. https://github.com/opendiffy/diffy/commit/e823c99196b29460b7e476b476ee9fd331fea677 Does the latest code not support setting these values?

puneetkhanduri commented 1 year ago

@HarshiShah : Yes. These settings are no longer supported. PR welcome.

puneetkhanduri commented 1 year ago

The maximum header size for request and response (the same setting) can now be configured via the maxHeaderSize setting as follows: -maxHeaderSize=33554432 This sets the max header size to 32 MB The maximum response size has also been tested upto 16 MB without needing any additional flags.

Please comment if still facing issues with the latest release and docker image.