sailthru / sailthru-node-client

Sailthru Node.js client
http://docs.sailthru.com
MIT License
17 stars 15 forks source link

Client doesn't return the same error codes as the api #42

Closed samcackett closed 3 years ago

samcackett commented 3 years ago

Hi, we're using the sailthru-client node.js library and not seeing the same error codes as the API.

For example if I look up a user and they don't exist, according to the API docs I should receive an error code of 99 with a message saying the user doesn't exist. If I query the API directly, this works:

{
    "error" : 99,
    "errormsg" : "User not found with email: test@test.com"
}

If I make the same request within the client library, I receive:

{
"error": 0,
"errormsg": undefined,
"statusCode": 0,
}

It's really unhelpful not to receive responses that align with the API.

This library hasn't been updated in 2 years, is it no longer supported? Could you either fix this and release an update, or let me know if this library is no longer supported and we'll just make API requests instead.

Thanks,

Sam

georgekliao commented 3 years ago

@samcackett this client library is supported. We'll look into this bug and get back to you. Thanks for pointing this out.

ben833 commented 3 years ago

Hi @samcackett I tried to replicate what you're seeing with:

sailthru.apiGet('user', {
    id: 'hey@nothing.com'
}, (err, response) => {
    console.error('Error: ', err);
    console.log('Response: ', response);
});

And got this:

Error:  {
  statusCode: 400,
  error: 99,
  errormsg: 'User not found with email: hey@nothing.com'
}
Response:  { error: 99, errormsg: 'User not found with email: hey@nothing.com' }

Can you provide the snippet where you're using the library? It looks like what you pasted was the err argument to the callback. Can you provide the response as well please?

samcackett commented 3 years ago

Hey both, thanks for the prompt responses and for looking into it.

We're using the method getUserByKey, not sure if that handles errors differently?:

const client = createSailthruClient(
    sailthruClientApiKey,
    sailthruClientSecret,
  )

  const getUser = async (email: string): Promise<SailthruReminderResponse> =>
    new Promise(resolve => {
      logger.debug('Fetching reminders data from Sailthru')
      client.getUserByKey(
        email,
        'email',
        {
          vars: '1',
        },
        (error: SailthruError | null, response: SailthruResponse) => {
          if (error) throw new Error(`Sailthru error: ${error.errormsg}`)
          logger.debug('Successfully fetched Sailthru reminders data')
          resolve((response as { vars: SailthruReminderResponse }).vars)
        },
      )
    })

Thanks,

Sam

ben833 commented 3 years ago

Hey Sam,

It appears that the throw in the callback of getUserByKey is causing this issue. Can you try using reject instead? With throw, the error is caught within the library, which sets error and statusCode to 0, effectively overwriting the values from the API. This code snippet allowed me to get the error message from the API:

const getUser = async (email: string): Promise<SailthruReminderResponse> =>
  new Promise((resolve, reject) => {
    logger.debug('Fetching reminders data from Sailthru')
    client.getUserByKey(
      email,
      'email',
      {
        vars: '1',
      },
      (error: SailthruError | null, response: SailthruResponse) => {
        if (error) {
          // WAS: throw new Error(`Sailthru error: ${error.errormsg}`)
          reject(`Sailthru error: ${error.errormsg}`)
        }
        else {
          logger.debug('Successfully fetched Sailthru reminders data')
          resolve((response as { vars: SailthruReminderResponse }).vars)
        }
      },
    )
  })

async function execute() {
  try {
    await getUser('test@test.com')
  }
  catch (error) {
    console.error(error)
  }
}

execute()

Caused this to show in the console:

Sailthru error: User not found with email: test@test.com
samcackett commented 3 years ago

@ben833 I forgot to say this worked and thank you! Apologies for the delayed reply