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

chore: Change debug level to info as this logging is frequent in harmless cases #13400

Merged
merged 4 commits into from
May 18, 2022

Conversation

TatuLund
Copy link
Contributor

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 15 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

github-actions bot commented Mar 31, 2022

Unit Test Results

   967 files  ±0     967 suites  ±0   46m 27s ⏱️ - 1m 56s
6 279 tests ±0  6 231 ✔️ ±0  48 💤 ±0  0 ±0 
6 504 runs   - 3  6 450 ✔️  - 3  54 💤 ±0  0 ±0 

Results for commit 30d5c6c. ± Comparison against base commit d32ec6b.

♻️ This comment has been updated with latest results.

@Artur-
Copy link
Member

Artur- commented Mar 31, 2022

Should this even be info or is it debug logging?

Copy link
Contributor

@mshabarov mshabarov left a comment

Choose a reason for hiding this comment

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

@TatuLund I had an another and more deep look into AbstractRpcInvocationHandler::handle method. Changing warn to debug seems a fine solution, because:

  1. When the node does not exist on the server state tree, the server basically has no information why, therefore it cannot decide whether log the message and what level to choose, and what do with it in the end.
  2. When the node is detached, IMO it doesn't make sense to figure out why it is detached. We can probably check if the node corresponds to HasValue component, but it is a "private" case when such a warnings occur.
  3. As Artur said, and I agree with him, these warn message doesn't give a clear understanding of what is broken or what's wrong with the caller's code. But it can be useful for development, debugging or anomaly investigation.
    So in short I propose to set debug level and apply this patch.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vaadin-bot
Copy link
Collaborator

Hi @TatuLund and @mshabarov, when i performed cherry-pick to this commit to 9.0, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 9691027
error: could not apply 9691027... chore: Change debug level to info as this logging is frequent in harmless cases (#13400)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

mshabarov pushed a commit that referenced this pull request May 19, 2022
mshabarov pushed a commit that referenced this pull request May 19, 2022
mshabarov pushed a commit that referenced this pull request Jun 10, 2022
taefi pushed a commit that referenced this pull request Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filtering grid causes Got an RPC for non-existent node warning
5 participants