T112286 (VE) and T111897 (CX) both reported errors in Parsoid which, after additional investigation, turned out to be errors in RESTBase. The reason why we all assumed it was a parsoid error is because both error-reporting code in both CX and VE/VRS still assume that these clients are directly talking to Parsoid. However, this is no longer the case. All client requests are now proxied through RESTBase. So, please update your error reporting code to pay attention to errors and dump the actual error messages and information about the error source. My concern is not that Parsoid is being blamed, but instead that we could be wasting time hunting down phantom errors.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Provide VRS objects with a name for more informative debugging/logging | mediawiki/core | master | +18 -1 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • GWicke | T112330 In current VRS deployment scenarios, error logging should not assume Parsoid is the source of all errors. | |||
| Resolved | Krenair | T112339 Change VisualEditor's error logging to not assume all VRS errors are Parsoid errors | |||
| Resolved | santhosh | T112565 ContentTranslation should not assume Parsoid is source of all VRS errors | |||
| Resolved | Catrope | T112566 Flow shouldn't assume Parsoid is source of all VRS errors |
Event Timeline
Comment Actions
For example, for T112286, VRS/VE-extension/VE should have received and reported the error in https://github.com/gwicke/restbase/commit/51bbb98aa0dd1a784c63a137f0478ba71f0acfb2 but that error got lost somewhere in the layers and it was reported as: "Something went wrong. parsoidserver-http: HTTP 400." even though the request never even got to Parsoid.
Comment Actions
@ssastry https://gerrit.wikimedia.org/r/#/c/237421/ is the first attempt to fix this.
Comment Actions
I left a bunch of comments on that patch -- there are still hardcoded references to restbase/parsoid there.
Comment Actions
Note that CX and VisualEditor share very similar code for creating the VRS object; tweaks for better error reporting probably can/should be kept in parallel as well.
The VRS object perhaps should grow a method to return a user-friendly name for the service (if it doesn't already have one).
Also/alternatively, the error messages are passing back the "error" string returned in the JSON object; we should perhaps add a mechanism to the Parsoid logger to ensure that the error string always contains a "parsoid" prefix. RESTBase should similarly ensure that any error string it returns is prefixed by "restbase".
Comment Actions
Also: I suspect the error message seen by the user is deliberately sanitized (ie, does not contain the error message provide by the service). However, the this->dieUsage call in the server-side API should end up in a log somewhere, shouldn't it? Maybe we are just looking in the wrong place?
Comment Actions
The VRS object perhaps should grow a method to return a user-friendly name for the service
It shouldn't really care what the backend is (that's a large part of the point of having a VRS abstraction in the first place), but we have a standard path that can be communicated.
Comment Actions
Change 238209 had a related patch set uploaded (by Cscott):
Provide VRS objects with a name for more informative debugging/logging
Comment Actions
Change 238209 merged by jenkins-bot:
Provide VRS objects with a name for more informative debugging/logging
Comment Actions
Based on the patches & inactivity since 2015, I believe this has been resolved. Please reopen if this is not the case.