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

[🐛 Bug]: With custom node example, node never see it's registered #1571

Closed
bhecquet opened this issue Jan 30, 2024 · 7 comments · Fixed by #1576
Closed

[🐛 Bug]: With custom node example, node never see it's registered #1571

bhecquet opened this issue Jan 30, 2024 · 7 comments · Fixed by #1576
Labels
bug Something isn't working

Comments

@bhecquet
Copy link
Contributor

What happened?

Hello,

using your sample code on https://www.selenium.dev/documentation/grid/advanced_features/customize_node/, I noticed that node never see it's registered
It's due to

protected DecoratedLoggingNode(Tracer tracer, URI uri, Secret registrationSecret) {
    super(tracer, new NodeId(UUID.randomUUID()), uri, registrationSecret);
  }

where we generate a new NodeId

in https://github.com/SeleniumHQ/selenium/blob/81865828a275ba384bc96fe10493cb1f7ee03f91/java/src/org/openqa/selenium/grid/node/httpd/NodeServer.java#L220

we get node status from node created with

Node node = LocalNodeFactory.create(config);

which has a different id than our custom node,
So when getting hub "node-added" registration event

https://github.com/SeleniumHQ/selenium/blob/81865828a275ba384bc96fe10493cb1f7ee03f91/java/src/org/openqa/selenium/grid/node/httpd/NodeServer.java#L143

ids do not match

Instead, constructor should reuse the id of the LocalNode

Node node = LocalNodeFactory.create(config);

DecoratedLoggingNode wrapper = new DecoratedLoggingNode(loggingOptions.getTracer(),
				node.getId(),
				uri,
				secretOptions.getRegistrationSecret());

What browsers and operating systems are you seeing the problem on?

chrome / windows

@bhecquet bhecquet added bug Something isn't working needs-triaging labels Jan 30, 2024
Copy link
Contributor

@bhecquet, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

@diemol
Copy link
Member

diemol commented Jan 30, 2024

@krmahadevan do you have a moment to have a look at this, please?

@krmahadevan
Copy link
Contributor

@diemol Sure. Will take a look.

@krmahadevan
Copy link
Contributor

@diemol - Observations made by @bhecquet are valid.

@bhecquet - Would you like to raise a PR for the documentation which makes this change and contribute ?

@bhecquet
Copy link
Contributor Author

bhecquet commented Feb 1, 2024

@krmahadevan , sure, will do that in a few days

bhecquet added a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
bhecquet added a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
bhecquet added a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
bhecquet added a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
bhecquet added a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
	modified:   customize_node.en.md
	modified:   customize_node.ja.md
	modified:   customize_node.pt-br.md
	modified:   customize_node.zh-cn.md
@bhecquet
Copy link
Contributor Author

bhecquet commented Feb 6, 2024

I've created the PR: #1576

diemol pushed a commit to bhecquet/seleniumhq.github.io that referenced this issue Feb 6, 2024
	modified:   customize_node.en.md
	modified:   customize_node.ja.md
	modified:   customize_node.pt-br.md
	modified:   customize_node.zh-cn.md
diemol pushed a commit that referenced this issue Feb 6, 2024
issue #1571: correct initialization of custom node
	modified:   customize_node.en.md
	modified:   customize_node.ja.md
	modified:   customize_node.pt-br.md
	modified:   customize_node.zh-cn.md

[deploy site]
@diemol diemol linked a pull request Feb 6, 2024 that will close this issue
6 tasks
@diemol
Copy link
Member

diemol commented Feb 6, 2024

Thanks for contributing, @bhecquet!

@diemol diemol closed this as completed Feb 6, 2024
selenium-ci added a commit that referenced this issue Feb 6, 2024
issue #1571: correct initialization of custom node
	modified:   customize_node.en.md
	modified:   customize_node.ja.md
	modified:   customize_node.pt-br.md
	modified:   customize_node.zh-cn.md

[deploy site] ef7b2e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants