fix: crash due to config packages key with missing value

[zermelo-wisen]

Fixes #285.

This PR addresses an issue where the AppMap agent crashes due to a malformed configuration file, specifically when the packages key value is missing. The changes ensure that such scenarios are handled gracefully, avoiding null pointer exceptions.

• Treat empty 'packages:' entry as empty array to prevent NPE. Ensures consistent behavior with when no 'packages:' entry or 'packages: []' is specified.
• Log and return null instead of System.exit(1) on 'load', since System.exit(1) is already done in 'initialize.'

Added tests to verify:

• Loading config with 'packages' key having no value.
• Error message for scalar 'packages' value.

Changes in AppMapConfig.java

ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
try {
  singleton = mapper.readValue(inputStream, AppMapConfig.class);
  // If an empty "package:" entry is specified, treat it as an empty
  // array to make the behavior consistent with cases where there is no
  // "package:" entry or where "package: []" is specified.
  if (singleton.packages == null)
      singleton.packages = new AppMapPackage[0];
} catch (IOException e) {
  logger.error("AppMap: encountered syntax error in appmap.yml {}", e.getMessage());
  return null;
}
singleton.configFile = configFile;
logger.debug("config: {}", singleton);

1. Handling Empty packages: Entry

   • The code now checks if the packages field is null after reading the appmap.yml file.
   • If packages is null, it is set to an empty array (AppMapPackage[0]).
   • This ensures consistent behavior when there is no packages: entry or when packages: [] is specified explicitly, preventing null pointer exceptions.

2. Error Handling Improvement

   • Instead of calling System.exit(1) 'load', the method now logs the error and returns null, since 'System.exit(1)' is already called in 'initialize' when the load returns null.

Changes in AppMapConfigTest.java

1. New Tests for packages Key
   • Added a test loadPackagesKeyWithMissingValue to verify that the configuration loads correctly when the packages key is present but has no value.
   • Added a test loadPackagesKeyWithScalarValue to verify that an appropriate error message is logged when the packages key has a scalar value instead of an array.

• Test for Missing Values:

@Test
public void loadPackagesKeyWithMissingValue() throws Exception {
  Path configFile = tmpdir.resolve("appmap.yml");
  final String contents = "name: test\npackages:\npath: xyz";
  Files.write(configFile, contents.getBytes());
  AppMapConfig.load(configFile, true);
}

• Test for Scalar Values:

@Test
public void loadPackagesKeyWithScalarValue() throws Exception {
  Path configFile = Files.createTempFile("appmap", ".yml");
  final String contents = "name:q test\npackages: abc\n";
  Files.write(configFile, contents.getBytes());
  String actualErr = tapSystemErr(() -> AppMapConfig.load(configFile, false));
  assertTrue(actualErr.contains("AppMap: encountered syntax error in appmap.yml"));
}

These tests ensure that the configuration loading behaves as expected for various malformed packages key scenarios, preventing crashes similar to the one reported.

[apotterri]

I don't think this is the right way to handle this case. Specifying a null value for package is an error. Instead of crashing with an NPE, the agent should detect this error and show the user an informative message.

[apotterri]

See my comment.

[zermelo-wisen]

I don't think this is the right way to handle this case. Specifying a null value for package is an error. Instead of crashing with an NPE, the agent should detect this error and show the user an informative message.

Actually, I reviewed the behavior and made it consistent with how the agent handles the absence of a packages entry or when it is specified as an empty array (packages: []). Now I changed it to show an informative error message and quit.

[appland-release]

🎉 This issue has been resolved in version 1.26.10 🎉

The release is available on:

• v1.26.10
• GitHub release

Your semantic-release bot 📦🚀