Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Avoid crashing on invalid @Plugin annotations #373

Closed
kmecpp opened this issue Jul 18, 2018 · 11 comments
Closed

Avoid crashing on invalid @Plugin annotations #373

kmecpp opened this issue Jul 18, 2018 · 11 comments

Comments

@kmecpp
Copy link

kmecpp commented Jul 18, 2018

Tried to launch my server after adding a Sponge plugin that depended on another Sponge plugin and the server crashed. Is this a Sponge issues?

[16:29:25 ERROR] [LaunchWrapper]: Unable to launch
java.lang.IllegalStateException: Expected state DEFAULT, but is DEPENDENCIES
        at com.google.common.base.Preconditions.checkState(Preconditions.java:721) ~[minecraft_server.1.12.2.jar:?]
        at org.spongepowered.server.launch.plugin.asm.PluginAnnotationVisitor.checkState(PluginAnnotationVisitor.java:61) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.plugin.asm.PluginAnnotationVisitor.visit(PluginAnnotationVisitor.java:71) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.a(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.a(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.a(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.a(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.accept(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.objectweb.asm.ClassReader.accept(Unknown Source) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.plugin.PluginScanner.scanClassFile(PluginScanner.java:345) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.plugin.PluginScanner.scanJar(PluginScanner.java:245) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.plugin.PluginScanner.scanDirectory(PluginScanner.java:173) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.plugin.VanillaLaunchPluginManager.findPlugins(VanillaLaunchPluginManager.java:66) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.VanillaServerTweaker.searchPlugins(VanillaServerTweaker.java:171) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.VanillaServerTweaker.injectIntoClassLoader(VanillaServerTweaker.java:94) ~[SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at net.minecraft.launchwrapper.Launch.launch(Launch.java:115) [launchwrapper-1.12.jar:?]
        at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]
        at org.spongepowered.server.launch.VanillaServerMain.main(VanillaServerMain.java:117) [SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]
        at org.spongepowered.server.launch.VersionCheckingMain.main(VersionCheckingMain.java:38) [SpongeVanilla-1.12.2-7.1.0-BETA-59.jar:1.12.2-7.1.0-BETA-59]

@limbo-app limbo-app added status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage version: 1.12 API: 7 labels Jul 18, 2018
@Zidane
Copy link
Member

Zidane commented Jul 18, 2018

@Minecrell can you look into this?

@kmecpp What plugins?

@kmecpp
Copy link
Author

kmecpp commented Jul 18, 2018

Two of my own plugins. It's possible I screwed something up but there's nothing about them in the stacktrace so I wouldn't know what the issue is

@stephan-gh
Copy link
Contributor

@kmecpp Can you share the JAR of your plugins? For some reason, it's confused about something in the @Plugin annotation.

@kmecpp
Copy link
Author

kmecpp commented Jul 19, 2018

Here are the jars. Note that the @Plugin annotation is autogenerated for the EnjinNewsSponge class and I'm using my own mcmod.info files.

mods.zip

@stephan-gh stephan-gh added the resolution: invalid This doesn't seem right label Jul 19, 2018
@limbo-app limbo-app removed the status: needs triage This label is automatically applied to new issues and pull requests to indicate they require triage label Jul 19, 2018
@stephan-gh
Copy link
Contributor

Note that the @Plugin annotation is autogenerated for the EnjinNewsSponge class

That sounds suspicious.

If you generate this automatically, you should only set the plugin ID in the @Plugin annotation. Everything else can be loaded from the generated mcmod.info file and will override the @Plugin annotation at runtime. There is no need to set plugin name, authors, version or dependencies in the plugin annotation.

To be exact however, the problem here is that you generate the dependencies of the @Plugin incorrectly. It's not a array of strings, it's an array of @Dependency annotations. Instead of:

@Plugin(
   id = "enjinnews",
   name = "EnjinNews",
   version = "1.0",
   description = "",
   authors = {"kmecpp"},
   dependencies = {"spongeapi@7.0.0", "Osmium"},
   url = ""
)

it should be something like:

@Plugin(
   id = "enjinnews",
   name = "EnjinNews",
   version = "1.0",
   authors = {"kmecpp"},
   dependencies = {
      @Dependency(id = "spongeapi", version = "7.0.0"),
      @Dependency(id = "Osmium"), // Note: Upper case plugin ID cannot exist
  }
)

... but as already mentioned this isn't necessary if you have it in mcmod.info.

@kmecpp
Copy link
Author

kmecpp commented Jul 19, 2018

Ahh thank you! Just saved me a lot of head banging

@stephan-gh
Copy link
Contributor

Additional suggestion: Consider using https://github.com/SpongePowered/plugin-meta to generate the mcmod.info file. It's already a dependency of SpongeAPI or you can depend on org.spongepowered:plugin-meta:0.4.1. Then create a new instance of PluginMetadata and use one of the methods in McModInfo to write it.

That way you don't need to know how the mcmod.info file looks like.

@kmecpp
Copy link
Author

kmecpp commented Jul 19, 2018

Thanks for the suggestion! I will take a look

@ryantheleach
Copy link
Contributor

Should an invalid dependencies specification in a plugin crash the server?

Can we add a sanity check + error message?

@ryantheleach ryantheleach reopened this Jul 19, 2018
@stephan-gh stephan-gh added status: accepted and removed resolution: invalid This doesn't seem right labels Jul 19, 2018
@stephan-gh stephan-gh changed the title Error starting server when using a plugin with dependencies Avoid crashing on invalid @Plugin annotations Jul 19, 2018
@ImMorpheus
Copy link
Contributor

ImMorpheus commented Jan 20, 2019

@Plugin(
   id = "enjinnews",
   name = "EnjinNews",
   version = "1.0",
   description = "",
   authors = {"kmecpp"},
   dependencies = {"spongeapi@7.0.0", "Osmium"},
   url = ""
)

This is a compile-time error. IMO the invalid label was the right solution. @ryantheleach

@stephan-gh
Copy link
Contributor

@ImMorpheus Yes, it was caused by generating bytecode directly in an invalid way. Nevertheless, SV should do more validation here and specifically say that plugin X is invalid, instead of crashing the whole server.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants