TS-43260 Profiler crash fix in case of invalid config options

Author: stahlbauer

CHANGELOG.md

@@ -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

agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java

@@ -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) {
@@ -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();
 
@@ -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;
 		} 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)
@@ -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;
 			}
@@ -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();
+		}
 	}
 
 	/**

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java

@@ -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());

agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java

@@ -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
 	 */