Skip to content

Commit

Permalink
Catch an exception due to incorrect pattern in Strings.format (#87132)
Browse files Browse the repository at this point in the history
Strings.format method, which is used heavily in logging with
Supplier should handle exceptions when a format is incorrect.
This will prevent a hard to catch mistakes to blow up in server.
Those mistakes are especially hard to detect in logging when a
code to create a message might be only executed when logger is debug
or trace. Which is not always the case in CI.

relates #87077 (comment)

relates #86549
  • Loading branch information
pgomulka authored May 30, 2022
1 parent faf8f32 commit 416a1b3
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/87132.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 87132
summary: Catch an exception when formatting a string fails
area: Infra/Logging
type: enhancement
issues: []
11 changes: 9 additions & 2 deletions libs/core/src/main/java/org/elasticsearch/core/Strings.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,18 @@ public class Strings {
/**
* Returns a formatted string using the specified format string and
* arguments.
*
* <p>
* This method calls {@link String#format(Locale, String, Object...)}
* with Locale.ROOT
* If format is incorrect the function will return format without populating
* its variable placeholders.
*/
public static String format(String format, Object... args) {
return String.format(Locale.ROOT, format, args);
try {
return String.format(Locale.ROOT, format, args);
} catch (Exception e) {
assert false : "Exception thrown when formatting [" + format + "]. " + e.getClass().getCanonicalName() + ". " + e.getMessage();
return format;
}
}
}
25 changes: 25 additions & 0 deletions libs/core/src/test/java/org/elasticsearch/core/StringsTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.core;

import org.elasticsearch.test.ESTestCase;

import static org.hamcrest.Matchers.equalTo;

public class StringsTests extends ESTestCase {

public void testIncorrectPattern() {
AssertionError assertionError = expectThrows(AssertionError.class, () -> Strings.format("%s %s", 1));
assertThat(
assertionError.getMessage(),
equalTo("Exception thrown when formatting [%s %s]. java.util.MissingFormatArgumentException. Format specifier '%s'")
);
}

}

0 comments on commit 416a1b3

Please sign in to comment.