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

Parsing failed for browser and os versions reported to stdout #20610

Open
Artur- opened this issue Dec 4, 2024 · 5 comments
Open

Parsing failed for browser and os versions reported to stdout #20610

Artur- opened this issue Dec 4, 2024 · 5 comments

Comments

@Artur-
Copy link
Member

Artur- commented Dec 4, 2024

Description of the bug

From logs of Start:

Browser engine version parsing failed for: mozilla 5.0 (windows nt 10.0; win64; x64) applewebkit 537.36 (khtml, like gecko) chrome 78.0.3904.97 safari 537.36 opr 65.0.3467.48 For input string: "a 5.0"
Browser major version parsing failed for: 17 sa For input string: "17 sa"
Browser major version parsing failed for: w For input string: "w"
Browser minor version parsing failed for:  For input string: ""
OS major version parsing failed for: /5 For input string: "/5"
OS minor version parsing failed for: 0 (android For input string: "0 (android"
OS minor version parsing failed for: 0) applewebkit/537 For input string: "0) applewebkit/537"

There seems to be two issues here:

  1. Parsing fails
  2. The error is reported to stdout or stderr instead of through a logger so it took some detective work to even figure out where they come from

Also as the full string that is being parsed is not logged in all cases, it makes things a bit difficult to fix

Expected behavior

No errors are logged without a logger and parsing succeeds

Minimal reproducible example

Versions

  • Vaadin / Flow version: 24.6 beta
@tepi
Copy link
Contributor

tepi commented Dec 5, 2024

As for issue 2, in this change this Artur guy explains why stderr is used instead of logging: 791b91c#diff-7354528a0f72607f3d6e1464b30a1fec4ca251c4cda835f7693bdd7cd5159e2bR568-R573

@Artur-
Copy link
Member Author

Artur- commented Dec 5, 2024

Always that guy. He is often wrong. Pretty good if nobody noticed or cared for 8 years though

@caalador caalador self-assigned this Dec 9, 2024
@caalador caalador moved this from 🟢Ready to Go to ⚒️ In progress in Vaadin Flow ongoing work (Vaadin 10+) Dec 9, 2024
caalador added a commit that referenced this issue Dec 10, 2024
Added new opera userAgent string.
Better logging for failures.
Better matching for version string.
Fixed android mistaken check.

Part of #20610
@caalador
Copy link
Contributor

I guess the stderr is because it is also used from the client and Logger is not available there.

What is the logging choice we should use for the shared package that would work both for the server and client?

@Artur-
Copy link
Member Author

Artur- commented Dec 10, 2024

There probably is no such thing and it should be logged in different ways on different sides

caalador added a commit that referenced this issue Dec 10, 2024
Added new opera userAgent string.
Better logging for failures.
Better matching for version string.
Fixed android mistaken check.

Part of #20610
@mshabarov mshabarov moved this from 🔖 Normal Priority (P2) to 🏗 WIP in Vaadin Flow bugs & maintenance (Vaadin 10+) Dec 10, 2024
@mshabarov mshabarov moved this from ⚒️ In progress to 🔎Iteration reviews in Vaadin Flow ongoing work (Vaadin 10+) Dec 11, 2024
mshabarov pushed a commit that referenced this issue Dec 12, 2024
* fix: browser parsing

Added new opera userAgent string.
Better logging for failures.
Better matching for version string.
Fixed android mistaken check.

Part of #20610

* Add ios firefox parsing

Add isIPad
Add testing userAgent strings
from json file.
@mshabarov mshabarov moved this from 🔎Iteration reviews to Done in Vaadin Flow ongoing work (Vaadin 10+) Dec 12, 2024
vaadin-bot pushed a commit that referenced this issue Dec 12, 2024
* fix: browser parsing

Added new opera userAgent string.
Better logging for failures.
Better matching for version string.
Fixed android mistaken check.

Part of #20610

* Add ios firefox parsing

Add isIPad
Add testing userAgent strings
from json file.
@mshabarov
Copy link
Contributor

We've merged the patch, let's keep it opened until we release a new version and use it for a while with Start to see if we have a proper logging now and any remaining errors.

mcollovati pushed a commit that referenced this issue Dec 12, 2024
* fix: browser parsing

Added new opera userAgent string.
Better logging for failures.
Better matching for version string.
Fixed android mistaken check.

Part of #20610

* Add ios firefox parsing

Add isIPad
Add testing userAgent strings
from json file.

Co-authored-by: caalador <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants