"Could not download mod list" - Github limits results to 1000 (there are >1000 mods now)

[unciv-loof]

Is there an existing issue for this?

• I have searched the existing issues

Game Version

4.18.1

Describe the bug

The github modlist downloader loads pages of 100 search results, but fails on the 11th page because only the first 1000 results are available through the github api.

My impression is that the download is always sorted by stars, so new mods with 0 stars may not be present in the first 1000 results. Especially discouraging for rookie modders.

https://api.github.com/search/repositories?q=%20topic:unciv-mod%20fork:true&sort:stars&per_page=100&page=11

{
  "message": "Only the first 1000 search results are available",
  "documentation_url": "https://docs.github.com/v3/search/",
  "status": "422"
}

[threadpool-daemon-2] [Github] [ERROR] Exception during GitHub download | java.io.IOException: Server returned HTTP response code: 422 for URL: https://api.github.com/search/repositories?q=%20topic:unciv-mod%20fork:true&sort:stars&per_page=100&page=11
...
at com.unciv.logic.github.Github.download(Github.kt:65)
at com.unciv.logic.github.Github.download$default(Github.kt:57)
at com.unciv.logic.github.Github.tryGetGithubReposWithTopic(Github.kt:348)
at com.unciv.logic.github.Github.tryGetGithubReposWithTopic$default(Github.kt:340)
at com.unciv.ui.screens.modmanager.ModManagementScreen$tryDownloadPage$1.invokeSuspend(ModManagementScreen.kt:285)
at com.unciv.ui.screens.modmanager.ModManagementScreen$tryDownloadPage$1.invoke(ModManagementScreen.kt)
at com.unciv.ui.screens.modmanager.ModManagementScreen$tryDownloadPage$1.invoke(ModManagementScreen.kt)
at com.unciv.utils.ConcurrencyKt$launchCrashHandling$1.invokeSuspend(Concurrency.kt:89)
...

Steps to Reproduce

Open game.
Go to Mods tab.
Wait for 1 minute.
Error pops up briefly inside unciv, and stack trace in terminal.

Screenshots

Link to save file

No response

Operating System

Linux

Additional Information

This part may be relevant

[unciv-loof]

A quickfix for the crash would be to simply limit the download to 10 pages (i.e. not try to download more than 1000 results).
To ensure that all mods show up in the mod list, a temporary solution can be to additionally query the bottom 1000 starred mods, and merge the two lists. Though of course, that solution will face the same problem once >2000 mods exist.

Neither of my 0 star mods showed up in the mod list. But once I gave one of them a star, it was moved it into the top 1000 results and thus showed up:

[RobLoach]

A few options...

1. Have Unciv accept an optional GitHub Peronal Access Token, and then use that when making GitHub API requests

   curl --header "Authorization: Bearer $GITHUB_TOKEN"
   

2. Make a small GitHub-Pages powered aggregator that would output a JSON file of all of the Unciv Mods available, and have Unciv download a zipped mods.json manifest instead. Something like this:
   https://robloach.github.io/unciv-mods/

[touhidurrr]

Umm... why is this an issue? Do they not already have pagination?

Update: Oh ok, it does not work

[touhidurrr]

by the way, why do we need fork:true in the query

[touhidurrr]

Solution 3: Ignore this error for now (loads first 1k repos)

[touhidurrr]

By the way, we should refactor GitHubAPI to use ktor client, as the dependency is already there. I looked at the code and it is too hardcoded, Handling things like redirects and retries manually. With ktor, this just works:

object KtorGithubAPI {
    const val baseUrl = "https://api.github.com"

    private val client = HttpClient(CIO) {
        followRedirects = true
        install(HttpRequestRetry) {
            maxRetries = 3
            retryOnException()
        }
    }

    /**
     * wait for rate limit to end if any and returns true if there was any rate limit
     */
    @OptIn(ExperimentalTime::class)
    private suspend fun consumeRateLimit(resp: HttpResponse): Boolean {
        if (resp.status != HttpStatusCode.Forbidden && resp.status != HttpStatusCode.TooManyRequests) return false

        val remainingRequests = resp.headers["x-ratelimit-remaining"]?.toIntOrNull() ?: 0
        if (remainingRequests < 1) return false

        val resetEpoch = resp.headers["x-ratelimit-reset"]?.toLongOrNull() ?: 0
        delay(Instant.fromEpochSeconds(resetEpoch) - Clock.System.now())

        return true
    }

    /**
     * Make a ktor request. Stuff like rate limits, retries and redirects is handled automatically
     */
    private suspend fun request(
        maxRateLimitedRetries: Int = 3,
        block: HttpRequestBuilder.() -> Unit,
    ): HttpResponse {
        val resp = client.request(block)
        val rateLimited = consumeRateLimit(resp)

        return if (rateLimited) {
            if (maxRateLimitedRetries <= 0) return resp
            return request(maxRateLimitedRetries - 1, block)
        } else resp
    }
}

[unciv-loof]

I just realized I overlooked issue when creating this. It references closed issue, which has various suggestions for how to resolve.

[touhidurrr]

@RobLoach @yairm210, an alternative fix can also be building a mods list manually during build and shipping the json with the app each time we upload a new version.

[SomeTroglodyte]

I documented this a long time ago (#13177)... And still think allowing users to add a github PAT enabling GraphQL - bypassing this restriction along with a bunch of others like rate limits would be the way to go. Or move the mod "repository" entirely (as in akin to an apt repository, not as in a github repo).

[unciv-loof]

I documented this a long time ago (#13177)... And still think allowing users to add a github PAT enabling GraphQL - bypassing this restriction along with a bunch of others like rate limits would be the way to go. Or move the mod "repository" entirely (as in akin to an apt repository, not as in a github repo).

I suspect requiring a PAT will turn away more rookie modders. Github is already daunting enough for non-programmers.

[SomeTroglodyte]

turn away

I said allowing not requiring

... but that's exactly why all that code&effort for GraphQL died.

[touhidurrr]

@SomeTroglodyte, whats the problem with this: #13951 (comment)

[SomeTroglodyte]

@SomeTroglodyte, whats the problem with this: #13951 (comment)

Didn't read that... Well, no biggie, but accountability¹ and extra effort.

Footnotes

1. As in, such a listing might be seen as endorsement, or third parties feeling their rights disrespected might start to begin the takedown request aggression loop. ↩