recurly / recurly-js

Simple subscription billing in the browser
https://js.recurly.com/
MIT License
645 stars 138 forks source link

Cannot read property 'number' of undefined #309

Closed joneslloyd closed 7 years ago

joneslloyd commented 7 years ago

Hi there,

Even though Recurly is successfully injecting its iframe code for Expiry Month, Expiry Year, CVV and Credit Card Number (observable by clicking Inspect in Chrome), upon submission, the following JS console error appears:

screen shot 2017-01-11 at 14 08 36

Do you know why that may be? This happens regardless of whether the Credit Card Number field has anything entered into it (or not) at the time of form submission.

chrissrogers commented 7 years ago

Hmm unfortunately it's not enough to go on. If you click the link recurly.js?ver=1.0.0:7 it will take you to the point in code that originated the error, which may be helpful. Do you have a link as well to your implementation in progress?

joneslloyd commented 7 years ago

@chrissrogers That point in the code (once unminified by Chrome dev tools) is this: screen shot 2017-01-12 at 10 47 55

I guess this relates to these files:

You can see the issue here, if you click "Submit": http://gfrecurly.onpressidium.com/payment/

chrissrogers commented 7 years ago

Thanks for the detail @joneslloyd -- I'll look into this further and see if I can suss the bug out

joneslloyd commented 7 years ago

No problem! I've actually solved the above now, in the past few minutes weirdly! (I'd overlooked a variable declaration).

However, I'm now noticing that the error thrown / returned by the API is

{
code: "validation",
fields: [
  0: "number",
  1: "month",
  2: "year",
  3: "first_name",
  4: "last_name",
  5: "cvv"
],
message: "There was an error validating your request.",
name: "validation"
}

Even when I enter a real (but expired) credit card number, all fields are marked invalid (I would have that that at least number would be okay doing that?)

joneslloyd commented 7 years ago

In fact, I just entered card details for an active credit card (Number, Expiry, Month, Year, CVV) and the same error was also returned.

chrissrogers commented 7 years ago

Can you verify that the request being sent to the token endpoint contains your card data?

joneslloyd commented 7 years ago

Actually, it doesn't seem to (I've tried using Chrome's Network tab to see this -- Is there another way I'm overlooking? ) - I'm guessing it could be because my Number, Expiry, Month, Year and CVV fields are inputs rather than divs?

It doesn't seem to have any problem injecting the Recurly code inside an input though: screen shot 2017-01-13 at 20 06 53

If the issue is because of this, is it possible to allow input field types?

chrissrogers commented 7 years ago

Possibly, but I doubt it. Would you mind putting your form handling and tokenization code into a gist?

joneslloyd commented 7 years ago

No problem - Here you go:

https://gist.github.com/joneslloyd/f6754fd7d6616ab93080ca8eb210f07e

(I am logging the Recurly response error to the console)

chrissrogers commented 7 years ago

I would suggest checking this line here to see if it is identifying the correct form

joneslloyd commented 7 years ago

It is correctly identifying the form. If I add a console.log here:

console.log( $( '#gform_' + GFRecurlyObj.formId )[ 0 ] );
recurly.token( $( '#gform_' + GFRecurlyObj.formId )[ 0 ], function ( err, token ) {
  GFRecurlyObj.responseHandler( err, token );
} );

It prints the following to the console, which, when I mouseover, highlights the correct form: screen shot 2017-01-13 at 21 16 15

joneslloyd commented 7 years ago

Just for your information: There was an update to the Recurly API repository that I added to my codebase, but this doesn't seem to have helped. The issue above still stands.

chrissrogers commented 7 years ago

Just had a chance to look through your page. It's as you suspected -- the input fields are not displaying the hosted fields. You'll need to use container elements of some sort, like a div, instead of an input element. When you're typing on that page, you're typing into the input element and not the actual hosted field, thus recurly.js does not know about the card values.

joneslloyd commented 7 years ago

Hi @chrissrogers - Thanks!

I thought that could be the case, but interestingly/oddly when I Inspect in Chrome on the example, the original (not injected) inputs are there in the DOM and receive the input: https://kale-krate.herokuapp.com/subscribe-minimal

chrissrogers commented 7 years ago

Mm yes, thanks for pointing to that. The kale krate example has been retired as it only demonstrated Recurly.js v3, thus the injected fields are not present as they were added in v4. It's no longer referenced in the documentation or available on github, but I will have to take down the heroku app soon too. Sorry for the confusion.

chrissrogers commented 7 years ago

Heads up that I just took it offline :]

joneslloyd commented 7 years ago

Got it! Okay!

I wonder (in that case) if Recurly JS is right for me / my solution, as what I'm using (Gravity Forms) has a load of built-in listeners and validators that rely on the inputs being unchanged (at least to the degree where they change from inputs to divs).

I probably know the answer already, but worth asking: Is there any way you can add something to accommodate users who would like to use Recurly JS for compliance reasons, but are limited by the functional requirements of their setup in having to use inputs?

I wonder if having the injected DIVs hidden, and being updated on each keypress into the existing inputs on the page would work....

chrissrogers commented 7 years ago

Unfortunately the limitation is precisely compliance. Under PCI, the merchant (yourself) mustn't have direct access to customer credit card data. The provider (Recurly) has to accept the brunt of the PCI compliance requirements, thus we need to isolate that customer data.

While we can't adapt to every form library out of the box, I think you may be able to build a connector if you must use a certain form library. Recurly.js emits a bunch of data about form field state on the 'change' event.

But moreover, I think the best solution is to not use the form library for the customer's card data. With a bit of CSS, you could achieve an exact look and feel that matches the appearance of your other form fields. I think your customer experience could be exactly as it is -- don't see any limitations there.

joneslloyd commented 7 years ago

@chrissrogers Thanks for the info!

Are these events documented? And which object am I listening to, just the recurly one itself?

chrissrogers commented 7 years ago

Yep, you'd listed by doing something like the following

recurly.on('change', function (state) {
  // check state.fields
});

Check out the docs here

joneslloyd commented 7 years ago

Cheers

joneslloyd commented 7 years ago

Hi @chrissrogers -- Sorry, I've realised that due to cross-domain iframe rules, I can't do something like this for the credit card number:

Is it possible to achieve this any other way? ie, pass the credit card number to the recurly JS object, etc.?

chrissrogers commented 7 years ago

Unfortunately it wouldn't be possible. The intent of the iframe is to restrict your page from ever accessing the credit card number, thus fulfilling the PCI SAQ-A requirements. Accepting card data in the manner you're describing would unfortunately not comply. I definitely recommend you accept customer card data directly via the hosted fields.

joneslloyd commented 7 years ago

Got it, okay!

I'll proceed without Recurly JS for now, but if it's possible, one day, to use input elements as well as divs, I can jump back in and use it.

Thanks for your help!