newrelic / newrelic-android-agent

SDK to enable instrumentation of Android mobile apps in New Relic
Apache License 2.0
15 stars 15 forks source link

Reference `classFile` variable when logging exceptions in `ClassTransformWrapperTask` #274

Closed jamiesanson closed 2 months ago

jamiesanson commented 2 months ago

Came across an exception in one of our engineers builds today which looks a little like the following:

Caused by: groovy.lang.MissingPropertyException: Could not get unknown property 'classJar' for task ':app:newrelicTransformClassesForGoogleDebug' of type com.newrelic.agent.android.ClassTransformWrapperTask.
    at org.gradle.internal.metaobject.AbstractDynamicObject.getMissingProperty(AbstractDynamicObject.java:85)
    at org.gradle.internal.metaobject.AbstractDynamicObject.getProperty(AbstractDynamicObject.java:62)
    at com.newrelic.agent.android.ClassTransformWrapperTask_Decorated.getProperty(Unknown Source)
    at com.newrelic.agent.android.ClassTransformWrapperTask$_transformClasses_closure1$_closure2$_closure5.doCall(ClassTransformWrapperTask.groovy:101)
    at jdk.internal.reflect.GeneratedMethodAccessor2490.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at com.newrelic.agent.android.ClassTransformWrapperTask$_transformClasses_closure1$_closure2.doCall(ClassTransformWrapperTask.groovy:71)

Digging into the NewRelic agent Gradle Plugin source, I noticed there's a variable used in an catch-block which isn't actually defined. Falling in to this catch block would cause an unrelated exception in the build, which is a little misleading.

I've referenced the nearest equivalent to other logger.error calls in this class, but there are further improvements that could be added here, including passing the caught fileException to the logger. This would show the cause of the error in the Gradle stacktrace:

--- logger.error("[ClassTransform] [${classFile.path}] ${fileException.message}")
+++ logger.error("[ClassTransform] [${classFile.path}]", fileException)
CLAassistant commented 2 months ago

CLA assistant check
All committers have signed the CLA.