404 checker

[eugenefvdm]

As the ecosystem matures a number of older packages are starting to have issues. The one that could be auto detected are the ones that are giving 404s because links have not been maintained.

Perhaps there can be some kind of scraper on this site that runs through all the links and looks for 404s. If a 404 is found then either the owner is notified or a small minor badge is attached to the detail view of the package to indicate 404 issues.

It's a bit disheartening to look at a package only to find some key links one would rely on no longer exists.

[mattstauffer]

Love this idea.

[marcusmoore]

An attempt was made to solve this in #75 but didn't take into account rate-limiting so we disabled the command.

@cameronscott137, the creator of the PR, outlined a good flow that I generally agree with a few modifications:

1. Run a daily command to check that each package, including packages that have been marked as unavailable, returns a 200 response code
   • Do the actual http requests in individual jobs that are queued
2. If a package returns 200
   • Clear marked_as_unavailable_at if it's set
3. If a package returns 404
   • If marked_as_unavailable_at is not set yet, set it to now()
   • If marked_as_unavailable_at is set and was set 5 days ago, then send the author the first email
   • If marked_as_unavailable_at is set and was set 10 days ago, then send the author a reminder email
   • If marked_as_unavailable_at is set and was set 15 days ago, then disable the package and send the author an email with instructions on how to re-enable it

Regarding the actual rate-limiting, I believe we can take advantage of Laravel's job rate-limiting features to space out the jobs and ensure we're not hammering third party sites.

Importantly, before this command set in the scheduler and emails go out to offline packages, we need to allow users to delete packages. #81 is open to track that issue.

[cameronscott137]

@marcusmoore confirmed. Will attempt redemption this weekend.

Having achieved some distance from it, the only addition that I would like to make is an alert on the package edit screen, notifying the author that an issue was detected with their URL. Within that alert, I'd like to include a link for the author to reset the URL status without changing the stored URL

Right now, the PR is coded to assume a change in the URL, but that might not always be the case. Furthermore, it's not immediately clear from the UI that changing the URL is required to reset the status. That will—and evidently did—lead to author confusion.

[marcusmoore]

@cameronscott137

Will attempt redemption this weekend.

Don't stress about it too much! 😄

I totally agree about making it clearer to the author that changing the url is what is needed to fix the issue. I would be ok with allowing the author to reset the url status (if they had made the repo private or something and then wanted to republish it). We would just want to make sure we check the url when it's submitted to make sure it's not still a 404.

[cameronscott137]

@marcusmoore Right on. Will open another PR in a few days. Hope you're well!

[cameronscott137]

** hat in hand **

@marcusmoore just got tagged when you converted this to a discussion. Happy to get this across the finish line unless y'all have other plans. Let me know, and thanks!

[marcusmoore]

No worries at all @cameronscott137!

We don't have any concrete plans yet and I moved this to a discussion to have a place to talk about approaches. Our outline in my comment above is probably a good place to start 😄

Note: I have to follow up on my PR for allowing packages to be deleted (#146) before this feature can be fully shipped.

[cameronscott137]

Agreed @marcusmoore. This should be a good candidate for Laravel's default rate-limiter middleware:

// AppServiceProvider.php
use Illuminate\Support\Facades\RateLimiter;

public function boot()
{
    RateLimiter::for('check-package-urls', function ($job) {
        return Limit::perMinute(30); // or whatever we figure is a safe interval
    });
}

Once defined, we can attach the limiter to the CheckPackageUrlsForAvailability job, which is where we're actually doing the third-party pinging:

// CheckPackageUrlsForAvailability.php
use Illuminate\Queue\Middleware\RateLimited;
 
public function middleware()
{
    return [new RateLimited('check-package-urls')];
}

I haven't actually tried this in code directly, but I believe this should allow the following flow:

• The CheckPackageUrls job gets kicked off from the scheduler, fetches all available packages, and dispatches an instance of CheckPackageUrlsForAvailability for each package. (It's this step that caused the issues: we near-simultaneously kicked off hundreds of jobs, quickly hitting Github's own rate limits.)
• Our new rate limiter kicks in for each CheckPackageUrlsForAvailability job: if it detects more than 30 jobs per minute, it will hold the remaining jobs until the next minute and dispatch another chunk. This process will continue until all jobs have been dispatched.
• Once dispatched, each CheckPackageUrlsForAvailability job will follow the flow above.

I don't think we'll need it, but if we find ourselves wishing for more fine-grained control, Spatie also has a package that offers a bit more nuance:

// CheckPackageUrlsForAvailability.php
use Spatie\RateLimitedMiddleware\RateLimited;

public function middleware()
{
    $rateLimitedMiddleware = (new RateLimited())
        ->allow(30)
        ->everySeconds(60)
        ->releaseAfterSeconds(90);

    return [$rateLimitedMiddleware];
}

Finally, on line 34 of our CheckPackageUrlsForAvailability job, I'd like to expand that catch statement to explicitly listen for a 404 response code. If detected, we'll update the package status accordingly, but otherwise, we'll leave the package as-is. That should exclude 403s, 401s, or anything that Github (or another service) might throw in the event that a rate-limit is exceeded.

This should insulate us in case something goes awry, Github's rate limits change, etc., so that we don't accidentally email a whole bunch of people again.

[marcusmoore]

Thanks for writing this all out @cameronscott137. I agree with your overview and think we can use it as a roadmap for the feature.

At some point in the near future I'll follow up on #146 and then this feature will be ready to be written and/or enabled. Feel free to dive in if you're feeling like PRing it yourself or I'll pick it up down the road.

Thanks again 😄