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

Add pretty printed exception example + steps to analyze. Fixes #479 #745

Merged
merged 4 commits into from Aug 24, 2018

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Aug 19, 2018

Fixes #479

@ST-DDT ST-DDT added needs review The submission is ready and needs to be reviewed low priority Small issues like typos that don't have much of an impact labels Aug 19, 2018
@Spongy
Copy link

Spongy commented Aug 19, 2018

A preview for this pull request is available at https://cdn.rawgit.com/Spongy/SpongeDocs-PRs/4ca0db2/.

Here are some links to the pages that were modified:

Since the preview frequently changes, please link to this comment, not to the direct url to the preview.

Copy link
Member

@Inscrutable Inscrutable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse my essay. Lots of weird sentences, commas needed here and there, some typos, and a few things I could not quite decipher the meaning of.
I'd value some educated opinions on the class name tracing bits in the middle.
Great work ST-DDT!

Exceptions at Runtime
=====================

There are two kind of errors that might occur on a modded server, first are plugin internal errors such as a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest breaking it into two sentences. (ie. "...modded server. First, there are ...")
Also, "internal plugin errors" reads better imho.
I don't understand the end of the sentence "... a command execution." Can you please clarify this bit?


There are two kind of errors that might occur on a modded server, first are plugin internal errors such as a
``NullPointerException`` a command execution. These exceptions are logged using the default stacktrace mechanism from
Java. The other kind of error is the error during during Minecraft's ticking of worlds or entities. Sponge handles these
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other -> second
"error is the error" looks weird.
Too many durings.
May I suggest "The second kind of error occurs during the ticking of worlds or entities."
(do you specifically need to refer to Minecraft here? Is it specifically a vanilla code thing?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps separate it into a bullet list instead, providing visual separation of a list? Something like (feel free to change or whatever):


There are two kinds of errors that may occur on modded servers:

  • Plugin internal errors, such as NullPointerExceptions during command execution. These errors are logged using the standard Java error handler.
  • Errors during the ticking of worlds or entities. Sponge prints out more structured errors to provide as much information as possible.

Java. The other kind of error is the error during during Minecraft's ticking of worlds or entities. Sponge handles these
cases in a special way to provide as much information about those as possible.

Before we look into the details on who caused the exception. Make sure that you read all of the exception, sometimes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first sentence is incomplete. Also, who -> what. May I humbly suggest:
"Before we look into the details of what caused the exception, make sure that you read all of the error report. Sometimes the error is explained in the error report itself (or a few lines above or below it); sometimes it is indicated by a warning during startup."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to be humble, your feedback is always welcome.

This stacktrace contains the most important version numbers, as well some information about the phase the server was in.
In this case a ``NullPointerException`` has been thrown during ``EntityTickPhase``. At this point its important to check
which plugins are involved with the crash. This requires some research as you have to match the package names with mod
names (Also check the ``Caused by`` blocks).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like an afterthought (And it should not start with a capital anyway). Hopefully this does it better:
"... names; checking the Caused by blocks may also help."


* ``java`` classes can be ignored during the error search.
* ``net.minecraft`` is the vanilla Minecraft code. If only these elements are present, its either a Minecraft bug or a
really nasty one (I explain those later).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its -> it's
We try to avoid first-person wherever possible, so can you change (I explain ...) to (these are explained below).
-- As an afterthought: do we have a calibrated scale of Nasty ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there is no such scale. However the nasty scale is tightly coupled with the required effort to find/analyze/understand/fix the issue.

Copy link
Member Author

@ST-DDT ST-DDT Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "nasty" -> "hard to find"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your choices of {nasty < really nasty} are sufficient.

also point to a hosting platform such as ``github``. In those cases you should also take a look at the next parts.
After that there is usually something related to the mod itself. ``extendedaiplugin`` might be related to the plugin
name. Please be aware that the mod may use additional separator characters such as ``-`` or spaces or use a different
character case for some parts. However this strategy is not always accurate, even less so in mod packs or bundles.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"However" -> "Beware," (it is a kind of warning).

.. note::

If you encounter a bug its usually a good idea to create a backup first, then trying to reproduce it, then narrowing
it down by removing mods. Only then should you report the error. If the error occurs while only non Sponge mods are
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"If the error occurs while only non Sponge mods are present try removing Sponge." is a bit confusing.
I presume you mean to say something like "If the error occurs in the absence of Sponge plugins, ..."
Also note that you are now referring specifically to SpongeForge, as it's a bit harder to remove SpongeVanilla for testing.

to the mod authors first as they have good knowledge of the parts of code they are working with. However you can
always contact us through IRC or other means. Please provide logs for bug reports, if possible.

Nasty bugs: Minecraft modding uses some advanced technics such as Mixins and ClassLoaderTransformations, which means
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technics -> "... techniques,"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


Nasty bugs: Minecraft modding uses some advanced technics such as Mixins and ClassLoaderTransformations, which means
that although a Minecraft class has been reported as the cause, it does not mean the code executed inside is from
Minecraft itself. Sponge and other plugins hook into the native methods and execute their own code parts; such as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code parts -> "code elements" (or components, or just "code")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this get missed? Also, the semicolon needs to be a comma.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT not + Fixed

Nasty bugs: Minecraft modding uses some advanced technics such as Mixins and ClassLoaderTransformations, which means
that although a Minecraft class has been reported as the cause, it does not mean the code executed inside is from
Minecraft itself. Sponge and other plugins hook into the native methods and execute their own code parts; such as
posting events. In that case you have to do a blind search for the causing mod. Unfortunately they also tend to only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"... you must do a blind search for the responsible mod" (or use malfeasant instead of responsible ;) )
The last sentence is also a bit vague ... do you mean something like "These may also occur with some combinations of plugins." or "These often occur through interaction between some combinations of plugins." ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the last one matches best. My intention was:

  • Mod A alone => no issues
  • Mod B alone => no issues
  • Mod A+B together => issues

(And I don't want to start with: "If you name/install/load them in a particular order", which would be very very nasty)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether malfeasant is the right term for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll do, although it is slightly borrowed. Other suitable words are misbehaving, problematic, troublesome, contrary. It's an artistic choice :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> misbehaving

@ST-DDT
Copy link
Member Author

ST-DDT commented Aug 20, 2018

Excuse my essay. Lots of weird sentences, commas needed here and there, some typos, and a few things I could not quite decipher the meaning of.

No problem at all. If you write an essay about my contribution, then I'm sure that it was actually read.
That does not mean that the PRs without comments haven't been read.
(its this strange zone between feeling good about getting feedback and sorry about making mistakes)

Anyway I tend to make more (spelling/grammar) issues if I try to express something complex or something without an easy "do it that way" explanation.
Do you know an IDE with rst + good grammar support? (Other than copy pasting to M$ Word)


.. note::

If you encounter a bug its usually a good idea to create a backup first, then trying to reproduce it, then narrowing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its -> "it's" or "it is".
trying -> try
narrowing -> narrow

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@Inscrutable Inscrutable removed the needs review The submission is ready and needs to be reviewed label Aug 23, 2018
@Inscrutable Inscrutable merged commit 99437fc into stable Aug 24, 2018
@Inscrutable
Copy link
Member

I want to try to keep to a grammar that translates well. English can be a curious beast, permitting many unusual conjugations within "poetic license". So if we can keep explanations simple, stepwise, and relatively free from complex jargon, that should be sufficient. (I'm sometimes guilty of over-long and complicated technical explanations...)

@ST-DDT ST-DDT deleted the feature/debugging branch August 24, 2018 16:21
* ``net.minecraft`` is the vanilla Minecraft code. If only these elements are present, it's either a Minecraft bug or a
really nasty one (these are explained later).
* ``org.spongepowered`` is from Sponge itself, having only these and Minecraft packages present usually indicates a
Sponge bug (or a nasty one). The :doc:`bugreport` chapter explains where to report bugs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we find a better name for this? nasty bug sounds like an STD to me, maybe "or another coremod being present"

you can always contact us through IRC or other means. Please provide logs for bug reports, if possible.

Nasty bugs: Minecraft modding uses some advanced techniques such as Mixins and ClassLoaderTransformations, which means
that although a Minecraft class has been reported as the cause, it does not mean the code executed inside is from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"it does not mean the code executed is necessarily part of the Minecraft source and could have been added by a third party."


Nasty bugs: Minecraft modding uses some advanced techniques such as Mixins and ClassLoaderTransformations, which means
that although a Minecraft class has been reported as the cause, it does not mean the code executed inside is from
Minecraft itself. Sponge and other plugins hook into the native methods and execute their own code, such as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Sponge and other coremods"
reason: it's no longer just a plugin once you modify the vanilla source

Nasty bugs: Minecraft modding uses some advanced techniques such as Mixins and ClassLoaderTransformations, which means
that although a Minecraft class has been reported as the cause, it does not mean the code executed inside is from
Minecraft itself. Sponge and other plugins hook into the native methods and execute their own code, such as
posting events. In that case you have to do a blind search for the misbehaving mod. These often occur only through
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case can check your logs for loaded coremods and potentially find the culprit by removing them one by one. Though be warned the issue may only occur when certain coremods are loaded at the same time.

@phit
Copy link
Contributor

phit commented Aug 24, 2018

oh rip reviewed this way too late, lots of wording there I'm not quite happy with, do you want me to make a PR to change what I think should be changed and we can discuss there?

@ST-DDT
Copy link
Member Author

ST-DDT commented Aug 24, 2018

How did you manage to do that?

Well additional review are always welcome. IMO opening a PR with your fixes is a good solution.

@phit
Copy link
Contributor

phit commented Aug 24, 2018

i went through my notifications and just started commenting before seeing the purple merged message, I'll make a PR :)

phit added a commit that referenced this pull request Aug 24, 2018
Inscrutable pushed a commit that referenced this pull request Sep 7, 2018
* more debugging clarifications and rewrites

adds to #745

* commas, semicolon & docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low priority Small issues like typos that don't have much of an impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants