Don't log 4XX errors as error-level in our logs for right now #370
Conversation
}); | ||
|
||
// Only want to warn about 400s for now (maybe forever?) | ||
if (statusCode < 500) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Truthfully, I'm not sure if new relic picks these up, but I think better to err on the side of caution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i'm guessing it watches stderr
const log = ((statusCode >= 400 && logger.error) || | ||
(statusCode >= 300 && logger.warn) || | ||
const log = ((statusCode >= 500 && logger.error) || | ||
(statusCode >= 400 && logger.warn) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm betting on this definitely making a difference.
if (response.statusCode >= 400) { | ||
// Don't throw an error for 400s for now (maybe forever?) | ||
return formatApiError({ message: response.statusMessage }, value); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has the same effect of the previous implementation - formatApiError
expects an Error
instance and will take the message
prop from it without throw
ing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did this first and it I don't think it's helpful. Removed.
if (statusCode < 500) { | ||
console.warn(errorMessageString); | ||
} else { | ||
console.error(errorMessageString); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trivially DRYer alternative
const logError = statusCode < 500 ? console.warn : console.error;
logError(errorMessageString);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
OK, comments addressed. |
No description provided.