Skip to content

Conversation

Zabuzard
Copy link
Member

@Zabuzard Zabuzard commented Dec 1, 2022

Overview

This fixes a couple of bugs with link previews.

Major priority, because the feature is currently live and many tags dont work anymore due to it.

Fixed bugs

  1. Sometimes included trailing punctation, like http://www.example.com/, or http://www.example.com/.
  2. Did not respect escaped URLs, like <http://www.example.com/>
  3. The link detector has a lot of false positives, such as System.in being detected as http://System.in/. Now filters out everything that is not http on the original URL.
  4. Some websites use <meta name="..."> instead of <meta property="...">
  5. When an individual query failed with an exception, it crashed the whole query instead of being skipped. Solved by using .exceptionally(...) with a proper log.warn on each task individually.
  6. When the full query failed with an exception, it lead to not sending the actual /tag anymore (stuck forever). Solved by using .exceptionally(...) with a proper log.error on the surrounding all-task
  7. Forgot a proper timeout for bad websites. Now is 10 seconds per website.

@Zabuzard Zabuzard added bug Something isn't working priority: major labels Dec 1, 2022
@Zabuzard Zabuzard self-assigned this Dec 1, 2022
@Zabuzard Zabuzard marked this pull request as ready for review December 1, 2022 09:06
@Zabuzard Zabuzard requested review from a team as code owners December 1, 2022 09:06
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2022

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Zabuzard
Copy link
Member Author

Zabuzard commented Dec 2, 2022

gonna ship this today. merging with one approval

@Zabuzard Zabuzard merged commit 0587d3e into develop Dec 2, 2022
@Zabuzard Zabuzard deleted the bugfix/link_preview_stuck branch December 2, 2022 07:27
@Zabuzard Zabuzard mentioned this pull request Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: major

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants