You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/12/04 11:11:35 UTC
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1089
tryLoadClass: return optional.absent on NoClassDefFoundError
We encountered this error when there were a Java entity in `lib/dropins` that still referenced the old `brooklyn.entity.basic.StartableApplication" (i.e. it had not been recompiled, with its source code updated).
The consequence was that `GET /v1/catalog/applications` failed, so no catalog items were displayed in the web-console. Also, the exception gives no indication of which catalog item the error came from (we unpacked all the jars, and did a string search across them to find it!).
With this fix, we'll get the log.debug that says which catalog item the error came from. The catalog item will be excluded from the list, and the other catalog items returned.
Ideally I'd like something better than just a `log.debug` to tell the user of this serious problem. Not convinced that `JavaBrooklynClassLoadingContext` is the right place to log.warn etc though.
I suggest we merge this, and then think about how to log better in a subsequent PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn fix/tryLoadClass
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1089.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1089
----
commit 68cd3860d2428381c36721c1b322b1fec69da536
Author: Aled Sage <al...@gmail.com>
Date: 2015-12-02T16:41:41Z
tryLoadClass: return optional.absent on NoClassDefFoundError
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#discussion_r46667152
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java ---
@@ -82,6 +87,10 @@ private ClassLoader getClassLoader() {
try {
className = DeserializingClassRenamesProvider.findMappedName(className);
return (Maybe) Maybe.of(getClassLoader().loadClass(className));
+ } catch (NoClassDefFoundError e) {
+ String msg = "Invalid linkage in (transitive dependencies of) class "+className+": "+e.toString();
+ LOG.debug(msg);
--- End diff --
As you said in the comment I prefer to be warning.
Users are not supposed to use invalid classes in their apps.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#discussion_r46678059
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java ---
@@ -82,6 +87,10 @@ private ClassLoader getClassLoader() {
try {
className = DeserializingClassRenamesProvider.findMappedName(className);
return (Maybe) Maybe.of(getClassLoader().loadClass(className));
+ } catch (NoClassDefFoundError e) {
+ String msg = "Invalid linkage in (transitive dependencies of) class "+className+": "+e.toString();
+ LOG.debug(msg);
--- End diff --
It is, you are right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#issuecomment-161929248
LGTM. Tested in a production setup scenario.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#discussion_r46714780
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java ---
@@ -82,6 +87,10 @@ private ClassLoader getClassLoader() {
try {
className = DeserializingClassRenamesProvider.findMappedName(className);
return (Maybe) Maybe.of(getClassLoader().loadClass(className));
+ } catch (NoClassDefFoundError e) {
+ String msg = "Invalid linkage in (transitive dependencies of) class "+className+": "+e.toString();
+ LOG.debug(msg);
--- End diff --
I'll live with flooding the debug log, as folk should really fix he underlying problem.
Longer term, we could guard to only log.warn once about it for a given class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#issuecomment-162044594
Merging now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1089#discussion_r46670540
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java ---
@@ -82,6 +87,10 @@ private ClassLoader getClassLoader() {
try {
className = DeserializingClassRenamesProvider.findMappedName(className);
return (Maybe) Maybe.of(getClassLoader().loadClass(className));
+ } catch (NoClassDefFoundError e) {
+ String msg = "Invalid linkage in (transitive dependencies of) class "+className+": "+e.toString();
+ LOG.debug(msg);
--- End diff --
This can be called multiple times a second due to refreshes from the browser, doesn't it flood the logs?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: tryLoadClass: return optional.abs...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1089
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---