-
Notifications
You must be signed in to change notification settings - Fork 318
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
More than a million validation errors crashes the application. #199
Comments
@khiftikhar I think it is a good idea and I am a big fan of fail-fast. If we can make it configurable, I don't think anybody will complain about it. Would you like to submit a PR for this feature? Thanks. |
@stevehu sure, I can start looking into it. Do you have any suggestion for a good candidate to put the code? Or which classes might be good to start looking at? I was thinking something like this,
|
@khiftikhar I think it is a good starting point. Previously, I was thinking to return on the first error, but make the max errors configurable might be better as other users might have that requirement. Also, it can prevent too many errors that crash the validator. We need to set a default maxErrors to a number big enough so that existing use cases won't be impacted. How about 100? From the processing efficiency perspective, it might be easier to handle one/all scenarios because we don't need to compare with the maxErrors in each validator. |
@stevehu What if the user is expecting more than 100 errors in one of his test cases? I was thinking to let the default value to be Integer/Long.MAX_VALUE. So it doesn't introduce any breaking changes. But personally, I would like to have default value <= 100.
Regarding this, can you specify what do you mean by that? I was thinking that we should stop the validator to do any processing if current |
I think it will definitely impact the performance if there are errors more than 100 and most use cases will have much less then 100. As you said, I would like a small number but I am afraid of changing the current behavior. What I mean regarding to the efficiency is that you have to compare the current number of the error in the error set with the maxErrors each time a new error is added. In the pure fail-fast, you return immediately when the first error is encountered. |
@stevehu Performance is the main reason most people use this library in the first place. So I completely understand and acknowledge your concern. So, we can have fail fast without maxErrors in the first version i.e. processing shall stop when a failFast() is configured to true and first validation error is encountered.
|
…y first validation error.
…y first validation error.
Hi,
We have an application which allows customer to send a json of 100 megabytes. In one case, we experienced that the customer sent a file in which every almost every field had validation exceptions. This validator gathers all the errors and report them at the end, but in this particular case, the there too+ many errors which caused the application to crash since it went out of memory. It would be nice to introduce a kind of fail early with a configurable threshold. Touching the threshold shall stop further processing and report errors gathered at that time.
The text was updated successfully, but these errors were encountered: