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

Make rejected promise always yield error instance #48

Merged
merged 1 commit into from
May 3, 2019

Conversation

ncjones
Copy link
Contributor

@ncjones ncjones commented May 2, 2019

This change makes all request errors compatible with VError which expects any cause to be an instance of Error.

For example:

try {
  return await dynamics.retrieveRequest({ collection, key });
} catch(e) {
  if (e.status == '404') {
    throw new VError({ cause: e, name: 'InvalidMessage' }, 'Equipment not found %j', { key });
  }
  throw new VError(e, 'Failed to get equipment %j', { key });
}

Without this change the VError constructor throws "cause is not an Error", masking the error. I think this issue should be fixed in VError too (TritonDataCenter/node-verror#66).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 96.221% when pulling c5b2e85 on ncjones:real-error-obj into 80cf08a on AleksandrRogov:master.

@AleksandrRogov
Copy link
Owner

Thank you for this PR, I see one issue here though: what if the CRM error object has inner error object? For example here. I guess we'll need to reassign keys recursively. Do you know if that "modern" spread operator works the latter way? If so, we need to use that syntax and I will add compilation commands to scale js version down as you suggested.

@ncjones
Copy link
Contributor Author

ncjones commented May 3, 2019

The nested objects will be copied by reference so it should just work as it is. AFAIK the spread operator can only be used when creating object literals. Object.assign could be used here for brevity though.

@AleksandrRogov AleksandrRogov merged commit 3ef9e2a into AleksandrRogov:master May 3, 2019
@ncjones ncjones deleted the real-error-obj branch May 4, 2019 00:38
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