Follow-up to #2472323: Move modal / dialog to query parameters
Problem/Motivation
Currently, DefaultExceptionSubscriber returns a JSON object with a single property, "error".
If, however, your response is caught by the JSON subscriber, it returns a JSON object with a
single property, "message". This is very inconsistent and unpredictable for consumers, as they don't know where to look for the detail of the error. (The property name changes depending on the severity and specificity of the problem, which is very non-obvious.)
Proposed resolution
Standardize on "message', because that is more widely used at the moment and so the less invasive change.
Remaining tasks
Commit it.
User interface changes
None.
API changes
Debatable; if we view the name of that property as an API, it's a minor change. If we don't, then there's no API change.
Crell says it's not an API change. :-)
Beta phase evaluation
Issue category | Bug because it's an error for us to be inconsistent in our errors. |
---|---|
Issue priority | Normal; there is only a small change with limited impact. |
Disruption | Should have no impact on anyone, except those who are relying on the format of an error message for REST responses. (Very very few people.) |
Comment | File | Size | Author |
---|---|---|---|
#8 | standardize_error-2486943-8.patch | 1.65 KB | lostkangaroo |
#4 | interdiff.txt | 1.98 KB | lostkangaroo |
#4 | standardize_error-2486943-4.patch | 2.75 KB | lostkangaroo |
Comments
Comment #1
Crell CreditAttribution: Crell at Palantir.net commentedComment #2
lostkangaroo CreditAttribution: lostkangaroo commentedI kept looking for something to make this complicated and seemed to only find one instance where error is used other than message. Seems that we can standardize on message pretty easily.
Comment #4
lostkangaroo CreditAttribution: lostkangaroo commentedThis takes care of the tests that failed due to the text of the property being changed.
Comment #5
lostkangaroo CreditAttribution: lostkangaroo commentedComment #6
Crell CreditAttribution: Crell at Palantir.net commentedMessage it is. Thanks!
Comment #8
lostkangaroo CreditAttribution: lostkangaroo commentedQuick reroll to fix the non apply. One test was fixed in the parent.
Comment #9
neclimdulwfm as well.
Comment #10
xjmThe issue summary doesn't seem to reflect the current state of the patch. I also don't quite grok why we're changing something that seems clearly an error into a non-error-type message; could we explain this in more detail? Thanks!
Comment #11
Crell CreditAttribution: Crell at Palantir.net commentedUpdated summary.
Comment #12
xjmThanks @Crell! The updated summary helps. I tweaked things a little (setting it to bug in accordance with what the summary says, and removing something about it possibly being major which didn't seem at all major anyway.) :)
I still don't entirely get why we would standardize on
message
(which seems semantically wrong) overerror
(which seems semantically correct) to minimize the disruption of something that (per the summary) is already not disruptive, but if @Crell and @neclimdul agree, well that's good enough for me. (And @Crell is the component maintainer, so that signoff helps reassure as well.) I reviewed withgit diff --color-words
and confirmed that the conversion oferror
tomessage
in code and test is the only change.I considered whether this should have a change record, but based on the summary it doesn't seem so.
This issue is a normal bug fix, and doesn't include any very disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed and pushed to 8.0.x.