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

fix(braintree): give better errors to our users #2490

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

jefflembeck
Copy link
Contributor

This parses braintree errors given to us by chargify and displays them for our users

This parses braintree errors given to us by chargify and displays them
for our users
@jefflembeck jefflembeck requested review from scottjehl, stoyan and tkadlec and removed request for scottjehl October 20, 2022 23:00
Copy link
Contributor

@stoyan stoyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet!

Maybe in a follow up we can make braintree-error-parser.js a module and load on demand in the event listener?

Comment on lines +435 to +436
var regExp = /\(([^)]+)\)/;
var errorCode = regExp.exec(errorText);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: const maybe?

Copy link
Contributor

@tkadlec tkadlec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

We should definitely work on more user-friendly alternatives to the messaging for some of these, but let's get this out so at least we have more actionable information and then we can massage that after.

@jefflembeck
Copy link
Contributor Author

Yeah, i'm going to fix this up today. Will get this in and will make it nicer.

@jefflembeck jefflembeck merged commit d528737 into master Oct 21, 2022
@jefflembeck jefflembeck deleted the better-braintree-errors branch October 21, 2022 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants