We have to check whether the get reactor is initialized in the pull backup reactor before checking its status. The initialization of this reactor happens in the writeResponse method so if we have an error in the initialize method, terminate's going to be called with get reactor uninitialized and that will lead to a crash.
Details
Details
make a request for pulling a backup without passing a backup id (missing any fields would trigger this I think).
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- fix-terminate-clb
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Sorry, the issue here was with the switch from phabricator.ashoat.com => phab.comm.dev. Had to update the webhook endpoint in Buildkite for the build status to get picked up by Phabricator.
Kicked off the builds again... hopefully everything works as expected. Will take a look to see if any other diffs are stuck in this state and kick them off again.
| services/backup/src/Reactors/server/PullBackupReactor.cpp | ||
|---|---|---|
| 140–141 ↗ | (On Diff #13371) | From the summary it looks like this->getReactor == nullptr means that there was an error - shouldn't we throw in that case? |
| services/backup/src/Reactors/server/PullBackupReactor.cpp | ||
|---|---|---|
| 140–141 ↗ | (On Diff #13371) | No. You're introducing a crash here. If get reactor is null, then reading anything from it in the next line causes a crash. I think it suffices to look at the status in terms of deciding whether an error occurred or not. |