Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Curseforge API Issues should not be fatal if pack is already installed #3159

Open
AngellusMortis opened this issue Nov 24, 2024 · 15 comments
Open

Comments

@AngellusMortis
Copy link

AngellusMortis commented Nov 24, 2024

Enhancement Type

Improve an existing feature

Describe the enhancement

If an AUTO_CURSEFORGE pack is already install, encountering Curseforge API issues (403 / Access to https://api.curseforge.com is forbidden or rate-limit has been exceeded) should not block startup of the Minecraft server.

I have had a pack installed for a bit over a week now. Restarted multiple times, everything has been fine. This morning, I scaled down the server to update a server config and now when I start it up I get:

[mc-image-helper] 10:06:47.853 ERROR : Invalid parameter provided for 'install-curseforge' command: Access to https://api.curseforge.com is forbidden or rate-limit has been exceeded. Ensure CF_API_KEY is set to a valid API key from https://console.curseforge.com/ or allow rate-limit to reset.

I have already tested my API key and everything locally and it appears to work. I think it is just Cloudflare temporarily flagging my IP or something. But still, a simple restart should not take down the whole server if the startup cannot communicate with Curseforge. The pack is already installed. Throw a warning it could not check for updates or something and let it continue.

@dominik-tietz
Copy link

I got the same problem. I am not able to start my server anymore.

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

Please provide your container config but be sure to hide the API key.

@AngellusMortis
Copy link
Author

I am not asking for support with the API key issue, just that it should not exit the pod if there is an API issue and the pack is already installed. It is really not just if there is a 403, but for any reason (similar to #2175).

If a pack has already been initially installed, the container should not break on restart for any reason related to networking issues.

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

I'm asking because I want to know if you're specifying a specific modpack version or not.

@AngellusMortis
Copy link
Author

I am.

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

This enhancement request is basically a duplicate of #2175.

@AngellusMortis
Copy link
Author

AngellusMortis commented Nov 24, 2024

I do agree they are essentially the root cause/fix.

That is specifically saying "Internet connection required" though. It should be reworded/renamed to something like "containers should not fail to restart if installed and there are any network issues". Since in this case, it is because of a bad response to Curseforge not a lack of Internet.

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

Right. The point is that there is a very small, but non-zero number of API calls needed to reconcile the requested modpack identifier/version and confirm it's the one previously installed.

I usually prefer not to change people's issue titles, but looking again it does seem like it could benefit from improved wording.

Edit

Their issue was encountering network requirements even before steps like cross checking CurseForge modpacks. So, yes, vaguely similar root causes, but distinct enough otherwise.

@AngellusMortis
Copy link
Author

AngellusMortis commented Nov 24, 2024

You could just write a simple file out say what the last "installed" pack was. Then if the old pack and the new pack match then

  • if the pack is not pinned (there can be an update / CF_FILE_ID is not set): do a warning instead of an exit
  • if the pack is pinned (CF_FILE_ID), do not even make network calls

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

It does do that. Look at the file .curseforge-manifest.json Again:

The point is that there is a very small, but non-zero number of API calls needed to reconcile the requested modpack identifier/version and confirm it's the one previously installed.

@AngellusMortis
Copy link
Author

AngellusMortis commented Nov 24, 2024

I do not see why you would need network calls then. If CF_SLUG and/or CF_FILE_ID match what is in the .curseforge-manifest.json, there is no need require the network requests. Unless there is something I am missing.

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

With debugs looks like it needs 3 API calls (but one was cached):

+ mc-image-helper install-curseforge --results-file=/data/.install-curseforge.env --force-synchronize=false --force-reinstall-modloader=false --overrides-skip-existing=false --modpack-page-url=https://www.curseforge.com/minecraft/modpacks/better-mc-forge-bmc4/files/5594853 '--exclude-mods=917285, 866084, 561885' --exclude-include-file=/image/cf-exclude-include.json
2024-11-24T18:32:00.201839971Z [mc-image-helper] 18:32:00.200 DEBUG : Loading cache index from ./.cache/curseforge/cache-index.json
[mc-image-helper] 18:32:00.643 DEBUG : JSON FETCH: uri=https://api.curseforge.com/v1/categories?gameId=432&classesOnly=true headers=[user-agent: itzg/mc-image-helper/1.40.4 (cmd=install-curseforge), x-fetch-session: be9dfbbf-71f2-4d8b-85cd-fc1867846e08, x-api-key: [redacted], accept: application/json, host: api.curseforge.com]
2024-11-24T18:32:00.680830762Z [mc-image-helper] 18:32:00.680 DEBUG : JSON FETCH: uri=https://api.curseforge.com/v1/mods/search?gameId=432&slug=better-mc-forge-bmc4&classId=4471 headers=[user-agent: itzg/mc-image-helper/1.40.4 (cmd=install-curseforge), x-fetch-session: be9dfbbf-71f2-4d8b-85cd-fc1867846e08, x-api-key: [redacted], accept: application/json, host: api.curseforge.com]
2024-11-24T18:32:00.698744804Z [mc-image-helper] 18:32:00.698 DEBUG : Getting mod file metadata for 876781:5594853
2024-11-24T18:32:00.703333304Z [mc-image-helper] 18:32:00.703 DEBUG : Loading cached content of getModFileInfo(876781,5594853) from ./.cache/curseforge/getModFileInfo/15ca0c40-0632-46fc-93db-f7a781787f49.json
2024-11-24T18:32:00.712359846Z [mc-image-helper] 18:32:00.711 INFO  : Requested CurseForge modpack Better MC [FORGE] 1.20.1 v29.5 is already installed for Better MC [FORGE] BMC4

Just noting if someone wants to help improve the code, it's located in https://github.com/itzg/mc-image-helper/tree/master/src/main/java/me/itzg/helpers/curseforge

@itzg
Copy link
Owner

itzg commented Nov 24, 2024

I got the same problem. I am not able to start my server anymore.

@dominik-tietz please open a separate issue and provide container config and logs.

@Djent-
Copy link

Djent- commented Dec 1, 2024

I wrote out a long comment related to the underlying 403 issue but then deleted it when I realized I had my CF_API_KEY formatted wrong. Escaping $ seems like an easy gotcha. When mc-image-helper encounters a 403, it would be a good user experience to sanity check the API key format and not reattempt the API call with the same invalid API key

@itzg
Copy link
Owner

itzg commented Dec 1, 2024

@Djent- unescaped dollar signs get pre-processed by docker as variable placeholders so the behavior is before is before and beyond the control of the image tooling. I'm also at the mercy of whatever API key format CurseForge chooses -- they might someday decide not to use the prefix with dollar signs in it. With that said, they've stuck to that format for a while now so I guess I could have the code inspect for that and print a warning about the unexpected format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants