Invalid href attribute value in WYSIWYG field causes fatal error during link checking

I have reviwed MR The fix looks good, need to improve the testInvalidUrl() test to be more explicit and precise, update the test to add a stronger assertion that specifically verifies the invalid URL was skipped

Core fix : injects LoggerInterface into LinkExtractorService, adds try/catch(\Throwable) around $link->save() in saveLink(), and registers the logger dependency in linkchecker.services.yml

Regression test : adds testInvalidUrl() to LinkcheckerLinkExtractorServiceTest with explicit assertions that valid URLs are saved and the malformed URL is skipped without crashing

Attached a related issue that was recently committed, but doesn't address when the space is within the host.

I also think the code can be simplified a bit more by just using the UrlHelper or the Uri class to check if the URL is valid rather than using our own logic.

There's also the linkchecker_watchdog_log() method which should avoid making changes to the services and allow this patch to easily apply against others.

I'll pick these changes up.

Simplified the changes and added a simple test case.

Attached a simplified version of the patch that works with less conflicts against other patches.

In particular the Utils import used in #3616064: Provide a way to check specific links immediately

Also experiencing this after semi recent guzzle changes. It is a huge inconvenience that the checker cron stops completely.

I have not spent much time looking through this module, however the fix looks simple enough and includes tests. Have applied and tested on a fresh install.

Created a node with the body field:

<p>
    This is a <a href="https://this is a test">link</a>
</p>

Observed the link was visible as unchecked after saving. Attempted to run drush linkchecker:check which failed

$drush linkchecker:check
> 
> In Uri.php line 76:
>                                   
>   Invalid host: "this is a test"  
>                                   
> 
> In Uri.php line 466:
>                                   
>   Invalid host: "this is a test"  
>                                   
> 

In ProcessBase.php line 155:
                    
  Output is empty.  

Applied patch and now the check completes and I can see the link in the link report correctly showing as "Invalid host: "this is a test" " in the error column.

I did wonder if we wanted to future proof a bit more for expanding which exception is caught? Given we just call getMessage on the exception, should we catch any Throwable to make this a bit more robust?

Aside from that though - I think this is an important fix and good for me. Setting to RTBC but may be worth thinking about the above.

Given we just call getMessage on the exception, should we catch any Throwable to make this a bit more robust?

I think the rest of the code is simple enough that it shouldn't need the generic catch. The only other exceptions that need to be caught would be within the request itself, which is being handled in #3469263: Links never checked.

Sounds good to me - thanks @codebymikey