somnambulist-tech / validation

A re-write of rakit/validation, a standalone validation library inspired by Laravel Validation
MIT License
44 stars 13 forks source link

Uncaught RuntimeException: Multiple File input without .* key #18

Closed danFWD closed 1 year ago

danFWD commented 1 year ago

Uncaught RuntimeException: Attribute "file_upload" has multiple files, use "file_upload.*" as the attribute key in... somnambulist/validation/src/Rules/Behaviours/CanValidateFiles.php on line 20

I though I would let you know this is not throwing a clean exception like other misconfigured fields do. Thank you.

dave-redfern commented 1 year ago

@danFWD Do you mean because this specific exception is a non-namespace RuntimeException and not a Validation\RuleException or the wording is not clear, or something else?

danFWD commented 1 year ago

Right, it’s an uncaught exception whereas when other fields are misconfigured, it returns an object you can send back as json. Thx

dave-redfern commented 1 year ago

I'm not sure I follow: if a rules parameters are invalid it will raise a Validation\Exceptions\ParameterException e.g.: if required_if is used without specifying at least one field it will throw an exception. Many other rules will do the same thing. These exceptions are not caught by this library.

I think you might be confusing the validation failed with "the validation component cannot process the current action".

A validation failure is because the data being validated fails to fulfill the requirements as configured in the rules. This will allow accessing an ErrorBag and you can handle this however you wish.

On the other hand: if the rule is mis-configured, then the validation cannot happen at all and an exception is raised. It is not possible to return an ErrorBag with ErrorMessages under these circumstances as validation could not proceed.

In the specific case of multiple file uploads, the field name must be specified using dot notation e.g. photos.* => uploaded_file or photos.main => uploaded_file, because the underlying _FILES array is structured differently depending on if there is a single file or multiple files. If this is not indicated, then the underlying data is not transformed correctly and instead of validating you get type errors.

Version 1.4.2 added an explicit exception to guard against this and to point to the actual issue: that you are attempting to validate an array of files without configuring the rules to indicate that. It is documented under the rule: https://github.com/somnambulist-tech/validation/blob/main/README.md?plain=1#L874

danFWD commented 1 year ago

You are right Dave, I am sorry for wasting your time. I just remembered that in the class I built to handle the request, I added checking for missing fields in the validation list. I send back a json response like this:

{
"success":0,
"error_type":"configuration",
"message":"Their were missing fields.",
"errors":{"bad_fields":["field_does_not_exist"]}
}

I’m dealing with a bad head cold right now and am not thinking clearly I guess. LOL

Do you have any advice on how I would go about handling the exception I presented to you earlier? This is to help provide feedback for other developers when they are building forms using this plugin. The public should never see this error.

Thanks

dave-redfern commented 1 year ago

That's alright - we all have bad days! I hope you get over your cold quickly. 🤗

Hmm, the exception is raised because something is fundamentally wrong in the setup, so it really needs to be fixed during dev. The "best" way (in my opinion) is to handle it during development and have a test case that can verify that your controller and validation is working. If using Symfony then you can fake calling the controller using the WebTestCase (https://symfony.com/doc/current/testing.html#submitting-forms). If you are using another framework then you would need to adapt for that case, or have a staging / test environment where you can deploy the code and actually test it before production.

Otherwise: you can use a try {} catch {} block around the validation but this would hide these errors and if not fixed would mean it would not work in production, so I really do not recommend doing this. You could adopt this and make a better error message, but again this is not necessarily something I actively recommend.

danFWD commented 1 year ago

Okay, thanks. My goal isn’t to hide misconfiguration errors but to make them more readable to anyone setting up the forms. Any error that comes from creating the validation array should be caught and returned as json so that it becomes a quicker diagnoses to the form designer. In my plugin, the form validation is encrypted and embedded on the front end and passed within the request headers using ajax. So you can see how it would be necessary to return a json response rather than a 500 server error. Thanks again for your hard work on this excellent module. I love Laravel and this about as close as it comes to that style of validation for other platforms.

dave-redfern commented 1 year ago

I can't take the credit; this library is based on rakit/validation. I merely forked it and tried to make it a bit better. 😁

I am glad you are liking it. I am happy to make changes to the error message(s) if you have suggestions for improvements or a way that things can be clearer.