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

Errors are being swallowed again, due to #3227 #3303

Closed
miracle2k opened this issue Aug 9, 2017 · 7 comments · Fixed by #3744
Closed

Errors are being swallowed again, due to #3227 #3303

miracle2k opened this issue Aug 9, 2017 · 7 comments · Fixed by #3744

Comments

@miracle2k
Copy link

What is the current behavior?

If my form's onSubmit handler returns a promise, and the onSubmit handler is passed as a form option, than the promise returned is rejected, that rejection is swallowed and I never see the error.

What is the expected behavior?

The error thrown is passed up to the browser console.

Sandbox Link

https://codesandbox.io/s/5w7Gmk6Z

Note how pressing Submit logs no error.

What's your environment?

Using 7.0.2.

Other information

Possibly related to #3179.

I tried to debug this and I believe the problem is listenToSubmit: https://github.com/erikras/redux-form/blob/171ed689cf998774c39bcd435d73dd8228cb7e02/src/createReduxForm.js#L657

It seems this was introduced recently in #3227, because before that submitFailed was throwing the error.

@Roman-Maksimov
Copy link
Contributor

Roman-Maksimov commented Aug 9, 2017

The problem with the #3227 (issue #1297) was about that the handleSubmit method mustn't throw any errors as it doesn't handle the errors anyhow. As a result the uncaught error appears and falls down the rest of the JS.
Don't know why do you want to have the uncaught error in your case (seems you used to integrate the bug into your logic).
If you need to catch the error, there is a onSubmitFailed method:

export default reduxForm({
  form: 'submitValidation', // a unique identifier for this form
  onSubmit: async () => {
      throw "test";
  },
  onSubmitFail: (errors, dispatch, submitError, props) => {
    console.log(submitError);
  }
})(SubmitValidationForm);

@miracle2k
Copy link
Author

miracle2k commented Aug 9, 2017 via email

@10xjs
Copy link
Contributor

10xjs commented Oct 3, 2017

@erikras @Roman-Maksimov I'm with @miracle2k on this. It's very misleading to consumers of redux-form to be converting any error rejected from the onSubmit to a resolved promise.

As a solution to #1297 - this a far worse compromise.

@minznerjosh
Copy link

minznerjosh commented Jan 2, 2018

The worst manifestation of this change is that, when using the imperative submit() API, the Promise returned by submit() will now fulfill with a rejection value, if a promise returned by onSubmit() rejects:

let Form = function Form(props) {
  const { handleSubmit } = props;

  return <form onSubmit={handleSubmit} />;
}
Form = reduxForm({ form: 'MyForm' })(Form);

let myForm = null; // will be a ref to <Form />

<Form
  ref={form => { myForm = form; }}
  onSubmit={() => Promise.reject(new Error('Bad news'))}
/>



// Now I'll imperatively submit the form:
myForm.submit().then((value) => { // handle fulfillment
  // This gets called with `new Error('Bad news')` ☹️
}, (reason) => { // handle rejection
  // This never gets called!
});

This behavior is at odds with the docs for the submit() method which states:

Submits the form. [You'd never have guessed that, right?] Returns a promise that will be resolved when the form is submitted successfully, or rejected if the submission fails.

@10xjs
Copy link
Contributor

10xjs commented Jan 5, 2018

For anyone looking for a temporary solution, it's possible to patch this behavior by wrapping reduxForm.

import {reduxForm, SubmissionError} from 'redux-form';

export function safeReduxForm(config, ...args) {
  return reduxForm({
    ...config,
    onSubmitFail: (error, dispatch, submitError, props) => {
      // Mirror the default logic of redux-form `handleSubmit` and don't
      // re-throw SubmissionError instances.
      // see: github.com/erikras/redux-form/blob/v7.0.4/src/handleSubmit.js#L69
      if (
        submitError && !(
          submitError instanceof SubmissionError &&
          submitError.errors
        )
      ) {
        if (config.onSubmitFail) {
          config.onSubmitFail(error, dispatch, submitError, props);
        } else {
          throw submitError;
        }
      }
      return error;
    },
  }, ...args);
}

Use as normal:

const WrappedComponent = safeReduxForm({form: 'form-name'})(FormComponent);

10xjs added a commit to 10xjs/redux-form that referenced this issue Jan 12, 2018
Fixes redux-form#3303

In the very likely case that an error is encountered during a submit callback we need to be able to reliably detect any failures.

This restores the correct behavior stated in the [documentation](https://redux-form.com/7.2.0/docs/api/reduxform.md/#-code-submit-promise-code-):

> Submits the form. [You'd never have guessed that, right?] Returns a promise that will be resolved when the form is submitted successfully, or rejected if the submission fails.

This will revert the change made in redux-form#3227, but throwing the error is a _far better_ compromise that benefits a _much larger set_ of redux-form users. It's possible to silence errors in your own code, but not possible to recover discarded errors from inside an isolated context in a lib.
erikras pushed a commit that referenced this issue Jan 16, 2018
Fixes #3303

In the very likely case that an error is encountered during a submit callback we need to be able to reliably detect any failures.

This restores the correct behavior stated in the [documentation](https://redux-form.com/7.2.0/docs/api/reduxform.md/#-code-submit-promise-code-):

> Submits the form. [You'd never have guessed that, right?] Returns a promise that will be resolved when the form is submitted successfully, or rejected if the submission fails.

This will revert the change made in #3227, but throwing the error is a _far better_ compromise that benefits a _much larger set_ of redux-form users. It's possible to silence errors in your own code, but not possible to recover discarded errors from inside an isolated context in a lib.
@erikras
Copy link
Member

erikras commented Jan 18, 2018

Published fix in v7.2.1.

@lock
Copy link

lock bot commented Jan 18, 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 Jan 18, 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

Successfully merging a pull request may close this issue.

5 participants