fix(#9799): add user-agent header to outbound #9818
Conversation
shared-libs/outbound/src/outbound.js
Outdated
| const getUserAgent = () => { | ||
| if (chtVersion === '') { | ||
| return secureSettings.getVersion() | ||
| .then(v => { | ||
| chtVersion = v; | ||
| const platform = os.platform(); | ||
| const arch = os.arch(); | ||
| return `${CHT_AGENT}/${v} (${platform},${arch})`; | ||
| }); | ||
| } | ||
| const platform = os.platform(); | ||
| const arch = os.arch(); | ||
| return Promise.resolve(`${CHT_AGENT}/${chtVersion} (${platform},${arch})`); | ||
| }; |
There was a problem hiding this comment.
There's a lot of duplication here, which I believe can be simplified like this:
| const getUserAgent = () => { | |
| if (chtVersion === '') { | |
| return secureSettings.getVersion() | |
| .then(v => { | |
| chtVersion = v; | |
| const platform = os.platform(); | |
| const arch = os.arch(); | |
| return `${CHT_AGENT}/${v} (${platform},${arch})`; | |
| }); | |
| } | |
| const platform = os.platform(); | |
| const arch = os.arch(); | |
| return Promise.resolve(`${CHT_AGENT}/${chtVersion} (${platform},${arch})`); | |
| }; | |
| const getUserAgent = async () => { | |
| if (!chtVersion) { | |
| chtVersion = await secureSettings.getVersion(); | |
| } | |
| const platform = os.platform(); | |
| const arch = os.arch(); | |
| return `${CHT_AGENT}/${chtVersion} (${platform},${arch})`; | |
| }; |
shared-libs/outbound/src/outbound.js
Outdated
| sendOptions.headers = { | ||
| Authorization: value | ||
| }; | ||
| sendOptions.headers.Authorization = value; |
There was a problem hiding this comment.
http headers must be lowercase in http2: https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2
I know we're not using http2 everywhere (yet), http1 headers are case-insensitive so might as well respect the stricter standard.
| sendOptions.headers.Authorization = value; | |
| sendOptions.headers.authorization = value; |
shared-libs/outbound/src/outbound.js
Outdated
|
|
||
| const setupUserAgent = () => { | ||
| return getUserAgent().then(userAgent => { | ||
| sendOptions.headers['User-Agent'] = userAgent; |
There was a problem hiding this comment.
http headers must be lowercase in http2: https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2
| sendOptions.headers['User-Agent'] = userAgent; | |
| sendOptions.headers['user-agent'] = userAgent; |
shared-libs/outbound/src/outbound.js
Outdated
| return setupUserAgent() | ||
| .then(() => auth()) | ||
| .then(() => { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug('About to send outbound request'); | ||
| const clone = JSON.parse(JSON.stringify(sendOptions)); | ||
| if (clone.auth && clone.auth.password) { | ||
| // mask password before logging | ||
| clone.auth.password = '*****'; | ||
| } | ||
| logger.debug(JSON.stringify(clone, null, 2)); | ||
| } | ||
| logger.debug(JSON.stringify(clone, null, 2)); | ||
| } | ||
|
|
||
| return request.post(sendOptions) | ||
| .then(result => { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug('result from outbound request'); | ||
| logger.debug(JSON.stringify(result, null, 2)); | ||
| } | ||
| }); | ||
| }); | ||
| return request.post(sendOptions) | ||
| .then(result => { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug('result from outbound request'); | ||
| logger.debug(JSON.stringify(result, null, 2)); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Can you please convert this to async / await?
shared-libs/settings/src/index.js
Outdated
| @@ -128,8 +131,26 @@ const getCouchConfig = (param, nodeName) => { | |||
| } | |||
| }; | |||
|
|
|||
| const getVersion = () => { | |||
There was a problem hiding this comment.
Please convert this to async / await.
Also I believe we should not be duplicating this code, we've had issues around versioning and I'm not comfortable keeping two sections that will return the version.
Can you maybe pass the version (which is static) to the outbound shared lib when you initialize it? Or move this code somewhere where it makes sense (my best idea here is the environment shared library), and call that in API as well?
adds user-agent header to outbound
User-Agent headers should include versions, which means a little extra stuff; it tries to get the CHT version in the same way as api/src/services/deploy-info.js, but that code is not directly available from shared-libs, so there is some small duplication.
Added getVersion() to shared-libs/settings. It doesn't quite fit there, but I don't think its worth it to make another separate shared-lib
We could also just omit the version if it adds too much mess, but I think these small additions are ok.
closes #9799