EandrewJones commented on code in PR #408:
URL: https://github.com/apache/flagon-useralejs/pull/408#discussion_r1480269429
##########
src/sendLogs.js:
##########
@@ -89,6 +89,12 @@ export function sendLogs(logs, config, retries) {
req.open("POST", config.url);
// Update headers
+ if( config.userId && config.password) {
Review Comment:
There's a bit of a code smell here. Silent, unexpected behavior may emerge.
If I set a password as well as an authHeader callback, the latter will
overwrite the basic authorization with whatever my function returns. We now
expose multiple ways to do the same thing: customer headers callback,
authHeaders callback, and this password setting.
I don't expect users to use both password _and_ the custom callback, but
exposing too many ways to do things can be confusing. _We really should force
all of this into the callback strategy pattern._ The problem is the callback
doesn't accept arguments and we have no way to persist the password unless we
store it on the config. However, as discussed, storing the password is also
poor practice. That said, so is basic auth...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org
For additional commands, e-mail: notifications-h...@flagon.apache.org
