feat!: Use Error#cause in GaxiosError

[danielbankhead]

Modern runtimes offer Error#cause - a standard way of defining the cause of another error.

This will also improving logging in some environments that handle cause in a special way.

Documentation:

• https://nodejs.org/api/errors.html#errorcause
• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause

Migration Guide

Use the cause property rather than error.

old:

const rootError = new GaxiosError().error;

new:

const rootError = new GaxiosError().cause;

Background

Dependencies (merge this PR after):

• feat: Support timeout for fetch and node-fetch v3 #660

Fixes #572 🦕

[generated-files-bot]

Warning: This pull request is touching the following templated files:

• README.md - README.md is managed by synthtool. However, a partials file can be used to update the README, e.g.: https://github.com/googleapis/nodejs-storage/blob/main/.readme-partials.yaml

[danielbankhead]

Note: Should merge #660 first.

[sofisl]

From the documentation, it seems like cause is in addition to, not instead of the error. Is it possible to set both in GaxiosError for it to not be a breaking change?

Also, seems like any is better than unknown here, WDYT?

[danielbankhead]

As GaxiosError is an error itself (with a few additional custom params) it would enable a chain of Errors to be appropriately logged along with each individual call stacks - providing a complete context for customers. The error param was a stop-gap, but often it would not be used properly. I think cleanly migrating would be a more optimal UX instead of both.

unknown is the appropriate type - cause could be anything, but should be verified through type checks.

[sofisl]

Perhaps a less destructive way then would be to add a deprecated tag to error, and simply continue to have it there? I'm concerned that it's a needlessly breaking change, especially since we're bundling a few other breaking changes in this release.

[danielbankhead]

A quick code search in googleapis will show virtually none of our libraries are using this property and it will be far more intuitive in the future to simply clean it up now.

[sofisl]

Right but perhaps it's being used by external users?

[sofisl]

(just for my curiosity) - why wouldn't we contstruct the error with e.cause vs. e.message?

[danielbankhead]

It allows GaxiosError to optionally have a root cause (say, the error is simple or purely on the gaxios level).

[sofisl]

why are we getting rid of err.name?

[danielbankhead]

That line could never be true as err.name is always GaxiosError

[sofisl]

Hm, this doesn't seem entirely relevant to this PR, I'm concerned it could have unintended consequences (even if it is a no-op). Would it be alright to leave it as a follow up?

[danielbankhead]

It’s not relevant, but is a one-line change. Not at all a big deal. Not worthy of a separate PR.

[sofisl]

Hm, those are often the most tricky! We've had this situation occur before. Until we have very robust integration testing, I think it would be wise to make small incremental changes. If we had good integration tests I'd feel better about it!

[sofisl]

I think we should put the error in a deprecation tag, and remove the one-line change checking for the error name and put it in a separate PR (hopefully after the next major, if it really is not-breaking). There are too many breaking changes in this upcoming major as it is (6 and counting!) and it makes for a difficult migration for our downstream libraries and any external users.