Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I got a UnsupportedDataTypeConfigException thrown because my config entity was badly formed. But the system log only told me the file where the exception was thrown, and the name of the entity. This leaves the developer having to grep the code for the entity name.
Beta phase evaluation
Issue category | Bug |
---|---|
Unfrozen changes | Unfrozen because it only changes a thrown exception to be more helpful |
Prioritized changes | The main goal of this issue is DX. |
Comment | File | Size | Author |
---|---|---|---|
#17 | interdiff-2267039-15-17.txt | 1022 bytes | aerozeppelin |
#17 | 2267039-17.patch | 2.16 KB | aerozeppelin |
#15 | interdiff-2267039-13-15.txt | 922 bytes | aerozeppelin |
#15 | 2267039-15.patch | 2.47 KB | aerozeppelin |
#13 | interdiff-2267039-11-13.txt | 3.32 KB | aerozeppelin |
Comments
Comment #1
joachim CreditAttribution: joachim commentedComment #2
jhedstromMakes sense to me. I updated the issue summary to include a beta phase evaluation.
Comment #4
joachim CreditAttribution: joachim commentedTestbot says 'The testbot client is probably malfunctioning.'. Huh.
Comment #8
joachim CreditAttribution: joachim commentedRerolled.
Comment #9
dawehnerImproving our exception, that is great!!
Comment #10
alexpottI like this patch.
We can test that the file is added to the message too. Just add an assertion in ConfigCRUDTest where the exception is tested.
Comment #11
joachim CreditAttribution: joachim commentedI've added a test of the exception message, but I'm not sure it's going to work.
The patch adds the details to the exception thrown in read(), but in the test it looks like what's being tested is an attempt to write a badly-formed config.
Comment #13
aerozeppelin CreditAttribution: aerozeppelin commentedTests for #11.
Comment #14
Xanocatch
must not be on the same line as the preceding closing curly bracket. Instead it must be on a new line.Apart from this coding style nitpick, the patch looks RTBC. I confirmed the tests cover the new behavior, and that no existing behavior has changed.
Comment #15
aerozeppelin CreditAttribution: aerozeppelin commentedChanges as per #14.
Comment #16
alexpottSafeMarkup::format() is deprecated and should not be used in exceptions anyway. sprintf() or just plain old string concatenation should be used. Using an escaping method in an exception message can cause double escaping as exception messages are only escaped at the time of output.
Comment #17
aerozeppelin CreditAttribution: aerozeppelin commentedChanges as per #16.
Comment #18
dawehnerNice! This is a really nice improvement
Comment #19
catchCommitted/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!