recurly / recurly-client-node

node.js library for Recurly's V3 API
https://developers.recurly.com
MIT License
44 stars 10 forks source link

Node Client - Invalid JSON returned from getPlan #72

Closed davidmcl closed 4 years ago

davidmcl commented 4 years ago

Describe the bug

On call to client.getPlan() the returned data is not valid JSON...the format is not the same as the raw api call. listPlans formats correctly.

To Reproduce

const recurly = require('recurly');
const client = new recurly.Client(config.apiKey);
...
let results = await client.getPlan(`code-${plancode}`);

The results in this case are:

**Plan** {
    id: 'd7hv7ygy1us3',
    object: 'plan',
    code: 'pro_annual',
    state: 'active',
    name: 'Pro Annual',
    description: 'Annual subscription to PRO, single-user account',
    intervalUnit: 'months',
    intervalLength: 12,
    trialUnit: 'days',
    trialLength: 0,
    totalBillingCycles: 1,
    autoRenew: true,
    accountingCode: 'pro',
    setupFeeAccountingCode: 'pro',
    taxCode: null,
    taxExempt: true,
    currencies: [ **PlanPricing** { currency: 'USD', setupFee: 0, unitAmount: 29 } ],
    hostedPages: **PlanHostedPages** {
      successUrl: '',
      cancelUrl: null,
      bypassConfirmation: false,
      displayQuantity: false
    },
    createdAt: 2015-07-27T15:49:00.000Z,
    updatedAt: 2019-06-24T21:29:24.000Z,
    deletedAt: null
  }

The 3 words surrounded with ** should not be in the results...it looks like the object names are being inserted.

Expected behavior

This is what is returned from the raw api call (no client)

{
    "id": "d7hv7ygy1us3",
    "object": "plan",
    "code": "pro_annual",
    "state": "active",
    "name": "Pro Annual",
    "description": "Annual subscription to PRO, single-user account",
    "interval_unit": "months",
    "interval_length": 12,
    "trial_unit": "days",
    "trial_length": 0,
    "total_billing_cycles": 1,
    "auto_renew": true,
    "accounting_code": "pro",
    "setup_fee_accounting_code": "pro",
    "tax_code": null,
    "tax_exempt": true,
    "currencies": [
        {
            "currency": "USD",
            "setup_fee": 0.0,
            "unit_amount": 29.0
        }
    ],
    "hosted_pages": {
        "success_url": "",
        "cancel_url": null,
        "bypass_confirmation": false,
        "display_quantity": false
    },
    "created_at": "2015-07-27T15:49:00Z",
    "updated_at": "2019-06-24T21:29:24Z",
    "deleted_at": null
}

Your Environment aws linux-2 VM, in docker container running node v12.13.1 npm v6.12.1

bhelx commented 4 years ago

@davidmcl I'm not sure how to reproduce this. Could you provide some code to help us understand?

the format is not the same as the raw api call.

The response is not a plain Object:

> plan.constructor.name
'Plan'
> plan instanceof recurly.Plan
true
bhelx commented 4 years ago

@davidmcl I think I see what you mean, i'm assuming you are seeing that when calling console.log on the plan?

> console.log(plan)
Plan {
  id: 'm23btfzr7lnp',
  object: 'plan',
  code: 'naeuxj3lfu',
  state: 'active',
  name: 'TocYwB3yBP',
  description: null,
  intervalUnit: 'months',
  intervalLength: 1,
  trialUnit: 'days',
  trialLength: 0,
  totalBillingCycles: 1,
  autoRenew: true,
  accountingCode: null,
  setupFeeAccountingCode: null,
  taxCode: null,
  taxExempt: false,
  currencies: [ PlanPricing { currency: 'USD', setupFee: 0, unitAmount: 0 } ],
  hostedPages: PlanHostedPages {
    successUrl: null,
    cancelUrl: null,
    bypassConfirmation: false,
    displayQuantity: false
  },
  createdAt: 2019-12-20T19:54:06.000Z,
  updatedAt: 2019-12-20T19:54:06.000Z,
  deletedAt: null
}

I believe that's just node's way of telling you the object is not a plain js object and has a defined type. You should still be able to treat it like a plain object though.

davidmcl commented 4 years ago

I did provide the code...it is literally the 3 statements provided, the only thing omitted is the console.log statement after the client call console.log(results);

RE: The response is not a plain Object: It is according to your documentation, or else your examples are all bad...maybe they are. The only way I could get the plain-object that I should get is by doing this...

const response = results.getResponse()
const plan = response.body;

...which should not be required according to your docs.

Here is your own examples from the documentation...looks like you are expecting a plain obect to me

try {
  const plan = await client.getPlan(planId)
  console.log('Fetched plan: ', plan.code)
} catch (err) {
  if (err instanceof recurly.errors.NotFoundError) {
    // If the request was not found, you may want to alert the user or
    // just return null
    console.log('Resource Not Found')
  } else {
    // If we don't know what to do with the err, we should
    // probably re-raise and let our web framework and logger handle it
    console.log('Unknown Error: ', err)
  }
}

re: I believe that's just node's way of telling you the object is not a plain js object and has a defined type.

Plain javascript is not typed and node isn't trying to tell me anything other than it's bad JSON returned. If it's not supposed to return a plain object, then you should indicate in the documentation that we have to use the getResponse() option, otherwise it looks to me that you need some code change.

According to the docs, the getResponse() was intended to get extra metadata, not as the normal procedure.

bhelx commented 4 years ago

@davidmcl sorry about the confusion.

The plan here is an instance of the Plan class https://github.com/recurly/recurly-client-node/blob/d7a3f8e622f50cc54d7891245fa3b3c45f6f8409/lib/recurly/resources/Plan.js#L37 which is inherited from Resource which is itself just a plain js object.

You should be able to use that plan object just like you would any other plain object. For instance:

plan.code // "pro_annual"
plan.currencies[0].currency // "USD"

I believe you are seeing those class names in console.log because node is telling you that they come from a higher prototype than a plain object.

You should be able to do anything else you'd want to do with that object as well such as JSON stringify:

JSON.stringify(plan) // '{"id":"m23m2z3vfqkw","object": .....

Again, sorry about the confusion. Let me know if that makes sense. I may still be missing something.

bhelx commented 4 years ago

@davidmcl Checking in to see if the issue is resolved.

bhelx commented 4 years ago

Closing for now. Feel free to re-open if you don't think the problem is resolved.