Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: generate "X-Request-ID" header #801

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ module.exports = (hermione) => {
Allows to use a custom `http`/`https`/`http2` [agent](https://www.npmjs.com/package/got#agent) to make requests. Default value is `null` (it means that will be used default http-agent from got).

#### headers
Allows to set custom [headers](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#headers) to pass into every webdriver request. These headers aren't passed into browser request. Read more about this option in [wdio](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#headers). Default value is `null`.
Allows to set custom [headers](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#headers) to pass into every webdriver request. These headers aren't passed into browser request. Read more about this option in [wdio](https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#headers). Default value is `{'X-Request-ID': '<some-uniq-request-id>'}`. One `X-Request-ID` is generated for the entire test run. It can be useful if you manage the cloud with browsers yourself and collect logs with requests.

#### transformRequest
Allows to intercept [HTTP request options](https://github.com/sindresorhus/got#options) before a WebDriver request is made. Default value is `null`. If function is passed then it takes `RequestOptions` as the first argument and should return modified `RequestOptions`. Example:
Expand Down
64 changes: 26 additions & 38 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"uglifyify": "^3.0.4",
"urijs": "^1.19.11",
"url-join": "^4.0.1",
"uuid": "^9.0.1",
"webdriverio": "8.13.4",
"worker-farm": "^1.7.0",
"yallist": "^3.1.1"
Expand Down
1 change: 1 addition & 0 deletions src/browser/existing-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
...this._config.meta,
};
}
Expand Down
9 changes: 8 additions & 1 deletion src/config/browser-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const _ = require("lodash");
const option = require("gemini-configparser").option;
const uuid = require("uuid");
const defaults = require("./defaults");
const optionsBuilder = require("./options-builder");
const utils = require("./utils");
Expand Down Expand Up @@ -305,7 +306,13 @@ function buildBrowserOptions(defaultFactory, extra) {
outputDir: options.optionalString("outputDir"),

agent: options.optionalObject("agent"),
headers: options.optionalObject("headers"),
headers: option({
parseEnv: JSON.parse,
parseCli: JSON.parse,
defaultValue: defaultFactory("headers"),
validate: value => utils.assertObject(value, "headers"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Теперь только object. null юзать нельзя

map: value => (value["X-Request-ID"] ? value : { "X-Request-ID": uuid.v4(), ...value }),
}),
transformRequest: options.optionalFunction("transformRequest"),
transformResponse: options.optionalFunction("transformResponse"),
strictSSL: options.optionalBoolean("strictSSL"),
Expand Down
2 changes: 1 addition & 1 deletion src/config/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module.exports = {
fileExtensions: [".js", ".mjs", ".ts", ".mts"],
outputDir: null,
agent: null,
headers: null,
headers: {},
transformRequest: null,
transformResponse: null,
strictSSL: null,
Expand Down
6 changes: 6 additions & 0 deletions src/config/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ exports.assertNonNegativeNumber = (value, name) => {
}
};

exports.assertObject = (value, name) => {
if (!_.isPlainObject(value)) {
throw new Error(`"${name}" must be an object`);
}
};

exports.assertOptionalObject = (value, name) => {
if (!_.isNull(value) && !_.isPlainObject(value)) {
throw new Error(`"${name}" must be an object`);
Expand Down
6 changes: 6 additions & 0 deletions test/src/browser/existing-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ describe("ExistingBrowser", () => {
assert.propertyVal(browser.meta, "browserVersion", "10.1");
});

it("should extend meta-info with 'X-Request-ID' from headers", () => {
const browser = mkBrowser_({ headers: { "X-Request-ID": "uniq-req-id" } });

assert.propertyVal(browser.meta, "X-Request-ID", "uniq-req-id");
});

it("should set meta-info with provided meta option", () => {
const browser = mkBrowser_({ meta: { k1: "v1" } });

Expand Down
1 change: 1 addition & 0 deletions test/src/browser/new-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe("NewBrowser", () => {
connectionRetryTimeout: 3000,
connectionRetryCount: 0,
baseUrl: "http://base_url",
headers: {},
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/src/browser/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function createBrowserConfig_(opts = {}) {
},
waitOrientationChange: true,
agent: null,
headers: null,
headers: {},
transformRequest: null,
transformResponse: null,
strictSSL: null,
Expand Down
66 changes: 53 additions & 13 deletions test/src/config/browser-options.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"use strict";

const _ = require("lodash");

const { Config } = require("src/config");
const defaults = require("src/config/defaults");
const { WEBDRIVER_PROTOCOL, DEVTOOLS_PROTOCOL, SAVE_HISTORY_MODE } = require("src/constants/config");
Expand Down Expand Up @@ -1626,34 +1625,75 @@ describe("config browser-options", () => {
});
});

["agent", "headers"].forEach(option => {
describe(option, () => {
it(`should throw error if "${option}" is not an object`, () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ [option]: "string" }));
describe("agent", () => {
it(`should throw error if "agent" is not an object`, () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ agent: "string" }));

Config.read.returns(readConfig);
Config.read.returns(readConfig);

assert.throws(() => createConfig(), Error, `"${option}" must be an object`);
});
assert.throws(() => createConfig(), Error, `"agent" must be an object`);
});

it("should set a default value if it is not set in config", () => {
it("should set a default value if it is not set in config", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_());
Config.read.returns(readConfig);

const config = createConfig();

assert.equal(config.agent, defaults.option);
});

it("should set provided value", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ agent: { k1: "v1", k2: "v2" } }));
Config.read.returns(readConfig);

const config = createConfig();

assert.deepEqual(config.browsers.b1.agent, { k1: "v1", k2: "v2" });
});
});

describe("headers", () => {
it("should throw error if option is not an object", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ headers: null }));

Config.read.returns(readConfig);

assert.throws(() => createConfig(), Error, `"headers" must be an object`);
});

describe("should generate 'X-Request-ID'", () => {
it("if value is not set in config", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_());
Config.read.returns(readConfig);

const config = createConfig();

assert.equal(config[option], defaults[option]);
assert.typeOf(config.browsers.b1.headers["X-Request-ID"], "string");
assert.isAbove(config.browsers.b1.headers["X-Request-ID"].length, 20);
});

it("should set provided value", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ [option]: { k1: "v1", k2: "v2" } }));
it("if it is not specified in value", () => {
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ headers: { k1: "v1" } }));
Config.read.returns(readConfig);

const config = createConfig();

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");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут используя sinon@4 невозможно застабать uuid.v4, так как это геттер. Нашел issue, что проблему починили в 15 версии. Но обновлять ее в рамках этого же PR не буду. Скорей всего куча тестов сломается.

Пробовал застабать и через proxyquire, но чтобы до этого uuid дотянутся из конфига нужно где-то тройную вложенность proxyquire написать. В итоге пока решил просто на длину проверять и наличие опции в хедере.

});
});

it("should not generate 'X-Request-ID' if it is specified by user", () => {
const headers = { "X-Request-ID": "my-req-id" };
const readConfig = _.set({}, "browsers.b1", mkBrowser_({ headers }));
Config.read.returns(readConfig);

const config = createConfig();

assert.deepEqual(config.browsers.b1.headers, headers);
});
});

["transformRequest", "transformResponse"].forEach(option => {
Expand Down
Loading