handle file upload errors #82

Merged
merged 1 commit into from Apr 17, 2014

Conversation

Projects
None yet
4 participants
@dbu
Member

dbu commented Nov 22, 2013

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? -
Tests pass? -
Fixed tickets #79
License MIT
Doc PR -

When uploading files through a form, there is no error handling. I thought we could use the upload file helper from the form type as well. This at least now gives a message that something went wrong and makes the upload file field marked as the field with error. however, it only gives the generic "invalid value" hint and does not show the ecxeption i throw. is there a way to better control that and give the exact error message like "file too big"?

- return $file;
+ try {
+ return $this->helper->handleUploadedFile($uploadedFile);
+ } catch(UploadException $e) {

This comment has been minimized.

@lsmith77

lsmith77 Nov 22, 2013

Member

missing space

@lsmith77

lsmith77 Nov 22, 2013

Member

missing space

@rmsint

This comment has been minimized.

Show comment
Hide comment
@rmsint

rmsint Nov 22, 2013

Contributor

There should be a way indeed to show the exact error message.

In the past I managed to get a custom error message attached to a field using the Symfony\Component\Validator\ExecutionContext specifying the property path and violation. I think this is not what you want to do here, however I wonder if the form framework triggers this somewere for the upload field and how it passes then the error?

Contributor

rmsint commented Nov 22, 2013

There should be a way indeed to show the exact error message.

In the past I managed to get a custom error message attached to a field using the Symfony\Component\Validator\ExecutionContext specifying the property path and violation. I think this is not what you want to do here, however I wonder if the form framework triggers this somewere for the upload field and how it passes then the error?

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 Mar 28, 2014

Member

ping

Member

lsmith77 commented Mar 28, 2014

ping

@dbu dbu changed the title from [WIP] handle file upload errors to handle file upload errors Apr 4, 2014

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu Apr 4, 2014

Member

this is now ready to merge. we still only get the generic invalid value message, seems the message from the transformer can't get forwarded to the form output and we would need a validator. however i have no clue where and how we would do that, so lets merge this as its a big improvement already.

Member

dbu commented Apr 4, 2014

this is now ready to merge. we still only get the generic invalid value message, seems the message from the transformer can't get forwarded to the form output and we would need a validator. however i have no clue where and how we would do that, so lets merge this as its a big improvement already.

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu Apr 4, 2014

Member

reverted the change in testing that created corebundle dependency and retriggered the build

Member

dbu commented Apr 4, 2014

reverted the change in testing that created corebundle dependency and retriggered the build

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu Apr 4, 2014

Member

okay, the tests seem to be a mess. but maybe i am just reveiling issues that went unnoticed before? would be great if somebody can sort this out for 1.1

Member

dbu commented Apr 4, 2014

okay, the tests seem to be a mess. but maybe i am just reveiling issues that went unnoticed before? would be great if somebody can sort this out for 1.1

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 Apr 17, 2014

Member

i really do not know this Bundle very well .. tests seem to be green now ..

Member

lsmith77 commented Apr 17, 2014

i really do not know this Bundle very well .. tests seem to be green now ..

dbu added a commit that referenced this pull request Apr 17, 2014

@dbu dbu merged commit 747a311 into master Apr 17, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@dbu dbu deleted the file-upload branch Apr 17, 2014

+ try {
+ return $this->helper->handleUploadedFile($uploadedFile, $this->class);
+ } catch(UploadException $e) {
+ throw new TransformationFailedException($e->getMessage(), $e->getCode(), $e);

This comment has been minimized.

@dbu

dbu Apr 17, 2014

Member

this does not end up as a validation error

@dbu

dbu Apr 17, 2014

Member

this does not end up as a validation error

This comment has been minimized.

@rmsint

rmsint Apr 17, 2014

Contributor

trying to catch up a bit to see if I can be of any help. The exceptions added here are very helpful I guess only now the user should be able to see them :-)

Imho some investigation has to be done how to properly catch the exceptions and convert to validation errors so the form component can pick them up and display them as error messages

@rmsint

rmsint Apr 17, 2014

Contributor

trying to catch up a bit to see if I can be of any help. The exceptions added here are very helpful I guess only now the user should be able to see them :-)

Imho some investigation has to be done how to properly catch the exceptions and convert to validation errors so the form component can pick them up and display them as error messages

This comment has been minimized.

@dbu

dbu Apr 17, 2014

Member

totally agree, created #97 for that and tried to reach out to webmozart. if you can help, its very much appreciated of course!

@dbu

dbu Apr 17, 2014

Member

totally agree, created #97 for that and tried to reach out to webmozart. if you can help, its very much appreciated of course!

This comment has been minimized.

@rmsint

rmsint Apr 17, 2014

Contributor

yes just figured out #97 was all about this ... I don't know the answer, unfortunately, but if I remember or find something that can help I will add it to ticket #97

@rmsint

rmsint Apr 17, 2014

Contributor

yes just figured out #97 was all about this ... I don't know the answer, unfortunately, but if I remember or find something that can help I will add it to ticket #97

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment