adobe / adobe/node-fetch-retry

consistent URL logging approach

Open
#83 5 comments 0 reactions 1 assignee Claimed by @alexkli View on GitHub
bug
Dominant language
JavaScript
Stars
44
Forks
16
PR merge metrics
No merged PRs in 30d

Description

From #55

> Note that `node-fetch-retry` is inconsistent and includes URLs in errors thrown (e.g. [here](https://github.com/adobe/node-fetch-retry/blob/3e512e5a6a892e0bd51c862d8fd1abdbdf1b4c57/index.js#L245)). It's fair to assume that all errors get logged downstream. So if the policy is to hide URLs by default, then they should not be included in errors either.
>
> Also it logs the FetchErrors (e.g. [here](https://github.com/adobe/node-fetch-retry/blob/3e512e5a6a892e0bd51c862d8fd1abdbdf1b4c57/index.js#L158)) which sometimes contain URL as well:
>
> ```
> FetchError failed with code: ECONNRESET; message: request to https://www.adobe.com/content/dam/shared/images/product-icons/svg/substance-3d-designer.svg failed, reason: read ECONNRESET
> Retrying in 103 milliseconds, attempt 1 error: FetchError, request to https://www.adobe.com/content/dam/shared/images/product-icons/svg/substance-3d-designer.svg failed, reason: read ECONNRESET
> ```
>
> With a custom logging option (#77) in place, there are two options:
> 1. log urls by default, and let clients with sensitive cases add a custom logger function that skips urls
> 2. do not log urls by default, and let clients who want to log urls add a custom logger function that does so
>
> But for convenience there could also be a third option:
>
> 3. add extra flag `logDetails` or `logUrl` that would be maybe off by default if security conscious

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.