Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
- PATCH version when you make backwards compatible bug fixes.

# Next version
- [fix] _agent_: The profiled application no longer crashes when the profiler configuration is invalid (e.g., missing `teamscale-user`). Instead, the application starts normally without coverage collection. The error message now also lists which specific options are missing.
- [feature] _teamscale-gradle-plugin_: Annotated tasks with `@DisableCachingByDefault` where caching can't be applied

# 36.4.0
Expand Down
18 changes: 12 additions & 6 deletions agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ public class PreMain {
private static final String ACCESS_TOKEN_ENVIRONMENT_VARIABLE = "TEAMSCALE_ACCESS_TOKEN";

/**
* Entry point for the agent, called by the JVM.
* Entry point for the agent, called by the JVM. If this method throws an exception, the JVM will abort the
* entire application. Therefore, configuration errors must be handled gracefully by returning normally, which
* allows the application to start without coverage collection.
*/
public static void premain(String options, Instrumentation instrumentation) throws Exception {
if (System.getProperty(LOCKING_SYSTEM_PROPERTY) != null) {
Expand Down Expand Up @@ -87,8 +89,6 @@ public static void premain(String options, Instrumentation instrumentation) thro
throw exception;
}
} catch (AgentOptionParseException e) {
getLoggerContext().getLogger(PreMain.class).error(e.getMessage(), e);

// Flush logs to Teamscale, if configured.
closeLoggingResources();

Expand All @@ -97,7 +97,9 @@ public static void premain(String options, Instrumentation instrumentation) thro
agentOptions.configurationViaTeamscale.unregisterProfiler();
}

throw e;
// Don't crash the profiled application due to a configuration error
// (see TS-43260). The error has already been logged in getAndApplyAgentOptions.
return;
Comment on lines +100 to +102
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The TI team discussed the topic and came to the (surprising) decision, that the agent should never crash the profiled application even if the agent is configured incorrectly. Can you please adjust the other places where we currently re-throw exceptions, document this decision in the code and adjust the tests if necessary.

} catch (AgentOptionReceiveException e) {
// When Teamscale is not available, we don't want to fail hard to still allow for testing even if no
// coverage is collected (see TS-33237)
Expand Down Expand Up @@ -158,7 +160,9 @@ private static Pair<AgentOptions, List<Exception>> getAndApplyAgentOptions(Strin
agentOptions = parseResult.getFirst();
} catch (AgentOptionParseException e) {
try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) {
delayedLogger.errorAndStdErr("Failed to parse agent options: " + e.getMessage(), e);
delayedLogger.errorAndStdErr(
"Failed to parse agent options: " + e.getMessage() + " The application should start up normally, but NO coverage will be collected!",
e);
attemptLogAndThrow(delayedLogger);
throw e;
}
Expand Down Expand Up @@ -206,7 +210,9 @@ private static void initializeLogging(AgentOptions agentOptions, DelayedLogger l

/** Closes the opened logging contexts. */
static void closeLoggingResources() {
loggingResources.close();
if (loggingResources != null) {
loggingResources.close();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,18 @@ private void readConfigFromTeamscale(
return;
}
if (!options.teamscaleServer.isConfiguredForServerConnection()) {
List<String> missingOptions = new ArrayList<>();
if (options.teamscaleServer.url == null) {
missingOptions.add("teamscale-server-url");
}
if (options.teamscaleServer.userName == null) {
missingOptions.add("teamscale-user");
}
if (options.teamscaleServer.userAccessToken == null) {
missingOptions.add("teamscale-access-token");
}
throw new AgentOptionParseException(
"Config-id '" + options.teamscaleServer.configId + "' specified without teamscale url/user/accessKey! These options must be provided locally via config-file or command line argument.");
"Config-id '" + options.teamscaleServer.configId + "' specified but the following required option(s) are missing: " + String.join(", ", missingOptions) + ". These options must be provided locally via config-file or command line argument.");
}
// Set ssl validation option in case it needs to be off before trying to reach Teamscale.
HttpUtils.setShouldValidateSsl(options.shouldValidateSsl());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,27 @@ private static AgentOptionsParser getAgentOptionsParserWithDummyLoggerAndCredent
return new AgentOptionsParser(new CommandLineLogger(), null, null, credentials, null);
}

/** Tests that config-id without teamscale-user reports the missing option by name. */
@Test
public void configIdWithMissingUserReportsMissingOption() {
assertThatThrownBy(
() -> parseAndMaybeThrow(
"config-id=test,teamscale-server-url=http://localhost:8080,teamscale-access-token=keyfoo"))
.isInstanceOf(AgentOptionParseException.class)
.hasMessageContaining("teamscale-user");
}

/** Tests that config-id with all required options missing reports all of them. */
@Test
public void configIdWithAllMissingOptionsReportsAll() {
assertThatThrownBy(
() -> parseAndMaybeThrow("config-id=test"))
.isInstanceOf(AgentOptionParseException.class)
.hasMessageContaining("teamscale-server-url")
.hasMessageContaining("teamscale-user")
.hasMessageContaining("teamscale-access-token");
}

/**
* Delete created coverage folders
*/
Expand Down
Loading