feat: generate "X-Request-ID" header #801
Conversation
src/browser/existing-browser.js
Outdated
| @@ -169,6 +169,7 @@ module.exports = class ExistingBrowser extends Browser { | |||
| return { | |||
| pid: process.pid, | |||
| browserVersion: this.version, | |||
| "X-Request-Id": this._config.headers["X-Request-Id"], | |||
| parseEnv: JSON.parse, | ||
| parseCli: JSON.parse, | ||
| defaultValue: defaultFactory("headers"), | ||
| validate: value => utils.assertObject(value, "headers"), |
There was a problem hiding this comment.
Теперь только object. null юзать нельзя
src/config/browser-options.js
Outdated
| parseCli: JSON.parse, | ||
| defaultValue: defaultFactory("headers"), | ||
| validate: value => utils.assertObject(value, "headers"), | ||
| map: value => (value["X-Request-Id"] ? value : { "X-Request-Id": uuid.v4(), ...value }), |
There was a problem hiding this comment.
Если пользователь выставил X-Request-Id, то свой не генерю.
Так же тут id-шник генерится и для воркеров и для мастер. Но у нас есть синхронизация конфигов и в итоге все запросы в selenoid/selenium/etc будут лететь с id от мастера. Генерация id занимает около 20мс.
Можно было бы немного упороться и прокидывать сгенеренный id в мастере во все воркеры. Но получилось бы довольно сложный код. Поэтому решил оставить так.
There was a problem hiding this comment.
а если пользователь укажет 'x-request-id'? нужно этот вариант тоже обработать
There was a problem hiding this comment.
а если пользователь укажет
'x-request-id'? нужно этот вариант тоже обработать
Так я же выше написал - "Если пользователь выставил X-Request-Id, то свой не генерю." Т.е. этот кейс я учел.
There was a problem hiding this comment.
Обсудили в чате. Переименовал X-Request-Id в X-Request-ID, так как в спецификации именно так этот хедер называется.
Если пользователь добавит x-request-id или еще какую-то вариацию, то этот хедер полетит as is в запросе. Т.е. переопределить можно только если явно указать X-Request-ID.
| assert.deepEqual(config.browsers.b1[option], { k1: "v1", k2: "v2" }); | ||
| assert.typeOf(config.browsers.b1.headers["X-Request-Id"], "string"); | ||
| assert.isAbove(config.browsers.b1.headers["X-Request-Id"].length, 20); | ||
| assert.equal(config.browsers.b1.headers.k1, "v1"); |
There was a problem hiding this comment.
Тут используя sinon@4 невозможно застабать uuid.v4, так как это геттер. Нашел issue, что проблему починили в 15 версии. Но обновлять ее в рамках этого же PR не буду. Скорей всего куча тестов сломается.
Пробовал застабать и через proxyquire, но чтобы до этого uuid дотянутся из конфига нужно где-то тройную вложенность proxyquire написать. В итоге пока решил просто на длину проверять и наличие опции в хедере.
DudaGod
force-pushed
the
HERMIONE-1228.add_x_req_id_header
branch
from
f8305fe
to
968d2f6
Compare
DudaGod
changed the title
DudaGod
force-pushed
the
HERMIONE-1228.add_x_req_id_header
branch
from
968d2f6
to
df02533
Compare
BREAKING CHANGE: "headers" in the config are now an object with a "X-Request-ID" by default
DudaGod
force-pushed
the
HERMIONE-1228.add_x_req_id_header
branch
from
df02533
to
35df5cc
Compare
BREAKING CHANGE: "headers" in the config are now an object with a "X-Request-ID" by default