There's a lot of complexity and redundancy in the way status is returned from the request to the server, especially the FakeServer. We seem to have a sort of sandwich approach where the two ends, syncer_proto_util.cc and loopback_server.cc seem to use the pair of HttpResponse::ServerConnectionCode* server_status and int64_t* response_code, but in the middle we have the fake_server* that use int* error_code and int* response_code.
It isn't clear how the mapping between error+response <=> sttatus+response should work. Things that largely seem wrong include server_status being completely dropped [1] and response_code of -2 being treated as successful [2].
[1] https://cs.chromium.org/chromium/src/components/sync/test/fake_server/fake_server.cc?l=105
[2] https://cs.chromium.org/chromium/src/components/sync/engine_impl/net/sync_server_connection_manager.cc?l=82
If all tests are successful, we seem to avoid these inconsistencies. However, during recent test failures I hit this DCHECK [3]. You can easily repro this by modifying the loopback_server.cc to return false inside HandleGetUpdatesRequest or HandleCommitRequest, which should result in a failure that is elegantly handled by SyncerProtoUtil::PostClientToServerMessage and friends.
[3] https://cs.chromium.org/chromium/src/components/sync/engine_impl/syncer_proto_util.cc?l=389
Comment 1 by pnoland@chromium.org
, Nov 15 2017