When running, bin/search index returns 0 regardless of status. Should return non-0 on failure so it can be handled by callers.
Description
Revisions and Commits
rP Phabricator | |||
D14174 | rP24845c70b918 Refine error behavior of `bin/search index` | ||
D14173 | rPc99508cfe26f Explain upstream attitudes toward CLI exit codes |
Related Objects
Event Timeline
Example:
root@iphab>/srv/phab/phabricator/bin/search index --all Run this workflow with "--background" to queue tasks for the daemon workers. Indexing 1 object of type USER. [ ] 0.0%[2014-08-28 21:39:40] PHLOG: 'Unable to index document PHID-USER-wbxngm5jjacogkt4vdro with engine PhabricatorSearchEngineElastic.' at [/srv/phab/phabricator/src/applications/search/index/PhabricatorSearchDocumentIndexer.php:57] [2014-08-28 21:39:40] EXCEPTION: (HTTPFutureHTTPResponseStatus) [HTTP/404] Not Found {"error":"IndexMissingException[[phabricatormain] missing]","status":404} at ...truncated output... Done.
root@iphab>echo $? 0
This is a bit complicated. We return nonzero on a "hard" error, like a bad type:
$ ./bin/search index --type ABCD echo Usage Exception: Nothing to index! $ echo $? 77
...or a bad object:
$ ./bin/search index D99999 Usage Exception: 'D99999' is not the name of a known object. $ echo $? 77
We consider some objects being un-indexable under --all to not be an error, because installs frequently have a small number of un-indexable objects, usually because at some point someone deleted something from the database manually. For example, if a repository is deleted from the database, revisions which belong to that repository will no longer be loadable and thus no longer be indexable. So, in practice, this situation most commonly corresponds to 99% of things working correctly and a handful of broken objects not getting indexed.
One issue is that we don't currently distinguish between a document-construction error (e.g., unable to load objects, usually because the install owner has been messing with the DB) and a document-submission error (e.g., unable to actually index the document when sending it to the search engine). We could reasonably consider the latter to be more severe, particularly if it occurs for every document we attempt to index.
One potential issue with treating submission errors as severe is that some document submission errors may currently be caused by T1191.
D14173 explains upstream attitudes toward exit codes in greater detail.
D14174 refines the exit code behavior of bin/search index by making it exit with an error code if all requested documents failed to index. I think this aligns both with the original scenario and with a consistent upstream worldview of what exit codes do. In particular, if your search backend is broken/misconfigured, we'll fail on every document and exit nonzero.