Skip to content
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

[BUG] onSubmitFail not called anymore on submitHandler promise rejection in v7.2.1 #3801

Closed
felixhayashi opened this issue Jan 31, 2018 · 7 comments

Comments

@felixhayashi
Copy link

felixhayashi commented Jan 31, 2018

Hi,

Are you submitting a bug report or a feature request?

bug report

What is the current behavior?

after moving from v7.2.0 to v7.2.1 the registered onSubmitFail handler is not called anymore if submitHandler rejects the promise.

What is the expected behavior?

onSubmitFail should be called if an an async submitHandler rejects the promise.

onSubmitFail
A callback function that will be called when a submission fails for whatever reason. It will be called with the following parameters:
https://redux-form.com/7.2.1/docs/api/reduxform.md/#-code-onsubmitfail-function-code-optional-

What's your environment?

Chrome

@AlimovSV
Copy link

AlimovSV commented Feb 5, 2018

@felixhayashi
Copy link
Author

felixhayashi commented Feb 6, 2018

@AlimovSV Yeah, that's the spot in the code that introduced this new behaviour.

In the latest change in the handleSubmit.js file, as you pointed out, error has been newly added to the boolean expression as a prerequisite for onSubmitFail being called (1a0660c#diff-28d8f38ee02f29d2bc406450f6c0d870).

The problem is that the error thrown in our forms on submission does not extend SubmissionError but is of a different type so redux form as of 7.2.1 doesn't count them as error anymore :(

The docs clearly state:

onSubmitFail
A callback function that will be called when a submission fails for whatever reason. It will be called with the following parameters:
https://redux-form.com/7.2.1/docs/api/reduxform.md/#-code-onsubmitfail-function-code-optional-

So I suggest reverting the line change that was introduced in v7.2.1 (i.e. if (error && onSubmitFail) back to if (onSubmitFail)).

Regards

@felixhayashi
Copy link
Author

felixhayashi commented Feb 6, 2018

@erikras would you be fine with a patch that changes this behaviour back to allowing any error to be thrown in order to trigger submitFailed (as it was in pre v7.2.1)? Otherwise, this would be quite a breaking change and I don't see much of a benefit resulting from this restriction either.

@naw
Copy link
Contributor

naw commented Feb 8, 2018

Yes, I agree this change (from #3723) should be reverted. It doesn't seem to have been a good PR for more than one reason. Also see #3821. It doesn't seem to me that @h1b9b fully grokked all of the nuances of shifting the handler to a catch among other things.

@naw
Copy link
Contributor

naw commented Feb 8, 2018

@felixhayashi perhaps the test suite could be improved to cover your situation, as I agree this is a breaking change in the expected behavior around onSubmitFail

@erikras
Copy link
Member

erikras commented Mar 2, 2018

Published fix in v7.3.0.

@lock
Copy link

lock bot commented Mar 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants