ash-jc-allen / favicon-fetcher

A Laravel package for fetching favicons from websites.
MIT License
173 stars 13 forks source link

How do we handle HTTP timeouts? #49

Closed sts-ryan-holton closed 1 year ago

sts-ryan-holton commented 1 year ago

Hi, I'm using the http driver here for fetching favicon. When a favicon can't be retrieved, the HTTP driver seems to wait for a long time which is slowing down my front-end, any chance we can customise the timeout?

ash-jc-allen commented 1 year ago

Hey @sts-ryan-holton! Sorry that you're having issues with the package.

When you say "a favicon can't be retrieved", what do you mean by this? Is this because the website doesn't have a favicon? Or it does have a favicon because the package can't find the favicon in the site's HTML?

If possible, do you have an example of one of the URLs that's taking a long time to load so I can reproduce the issue? 🙂

sts-ryan-holton commented 1 year ago

Yeah sure, basically, the Laravel HTTP facade has a timeout option, I've had a look through https://github.com/ash-jc-allen/favicon-fetcher/blob/master/src/Drivers/HttpDriver.php and there's not settings for specifying timeout, meaning, when a URL such as https://dfgdfgdfgdf.com/ is set it'll spend several seconds, in my case around 8 seconds trying to load the icon, Laravel's default timeout if not explicitly defined will default to 60 seconds, in my case that's too long so I need to change it.

My backend is separate from the front-end, interestingly I'm also using the cache option, which, still hangs even after trying to load the image from that URL that can't be found, is this right?

Here's my usage at my model level, also, seems like when the image can't be found, it's not using my fallback URL, luckily wrapping in a try/catch I can get a fallback this way.

(note I am importing Carbon etc, just reduced the data for Github here)

/**
 * The accessors to append to the model's array form.
 *
 * @var array
 */
protected $appends = [
    'favicon_url'
];

/**
 * favicon
 */
public function getFaviconUrlAttribute(): string
{
    try {
        $favicon = Favicon::withFallback('google-shared-stuff')
                    ->fetchAllOr($this->company_website_url ?? '', 'https://cdn-icons-png.flaticon.com/128/733/733579.png')
                    ->cache(Carbon::now()->addDays(2))
                    ->largest();

        return $favicon->getFaviconUrl();
    } catch (\Exception $e) {
        return 'https://cdn-icons-png.flaticon.com/128/733/733579.png';
    }
}
ash-jc-allen commented 1 year ago

That's really helpful, thank you!

I'm pretty snowed under with work at the moment, so I don't have a huge amount of time to look at this. But I'll try and do some investigating at the weekend to see if I can track anything down 😄

sts-ryan-holton commented 1 year ago

I think this service would suit the projects I'm working on, but, just some things to iron out I think.

It would also be great if their was a way to fetch favicons behind the scenes on some kind of queued event listener too.

mstroink commented 1 year ago

Some websites don't accept requests from curl / bots and purposely delays incoming connections (until timeout is reached). For now you could create your own copy of the http driver with a timeout set.

ash-jc-allen commented 1 year ago

Hey! Just a quick update on this one. I've just merged a PR (https://github.com/ash-jc-allen/favicon-fetcher/pull/67) that should add these timeout options for you. I'm hoping to get this released in the next day or so 🙂

ash-jc-allen commented 1 year ago

I've just released Favicon Fetcher v3.0 (https://github.com/ash-jc-allen/favicon-fetcher/releases/tag/v3.0.0).

This release includes two new config options that you can use for this: connect_timeout and timeout. There's a small section in the docs about these options (https://github.com/ash-jc-allen/favicon-fetcher/tree/v3.0.0#http-timeouts).

Hopefully this solves your issue, but if you notice anything not quite right, feel free to open another issue 😄