Page MenuHomePhabricator
Differential D10265

Make bot macro handler more graceful in the face of failure
ClosedPublic
Actions

Authored by epriestley on Aug 14 2014, 3:37 PM.
Tags
None
Referenced Files
F14063804: D10265.diff
Mon, Nov 18, 8:25 PM
F14053257: D10265.diff
Fri, Nov 15, 1:48 PM
F14042273: D10265.id24719.diff
Tue, Nov 12, 1:57 AM
F14038237: D10265.diff
Sun, Nov 10, 10:20 PM
F14023014: D10265.diff
Wed, Nov 6, 9:53 PM
F14008762: D10265.diff
Wed, Oct 30, 6:37 AM
F13977281: D10265.diff
Oct 18 2024, 5:56 PM
F13972308: D10265.id24714.diff
Oct 17 2024, 6:17 PM
View All Files
Subscribers
epriestley

Details

Reviewers
btrahan
cburroughs
Maniphest Tasks
T5883: PhabricatorBotMacroHandler handling of image lookup failures lacks grace
Commits
Restricted Diffusion Commit
rP40a4eeec77a9: Make bot macro handler more graceful in the face of failure
Summary

Fixes T5883. The first time we hit an error we'll continue forward; we only bail after the second time. Instead, check for an error immediately

Test Plan

HA HA HA DID NOT TEST HA HA HA HA

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley updated this revision to Diff 24714.Aug 14 2014, 3:37 PM
epriestley retitled this revision from to Make bot macro handler more graceful in the face of failure.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: btrahan, cburroughs.
epriestley added a task: T5883: PhabricatorBotMacroHandler handling of image lookup failures lacks grace.
Herald added a subscriber: epriestley. · View Herald TranscriptAug 14 2014, 3:37 PM
Harbormaster completed remote builds in B2225: Diff 24714.Aug 14 2014, 3:38 PM
cburroughs edited edge metadata.Aug 14 2014, 3:47 PM

Not sure what the convention is for logging in daemons, but should this get a log message instead of silent bail?

epriestley added a comment.Aug 14 2014, 3:51 PM

We could refine this behavior, but the bot stuff is a wasteland of low-quality code right now. This is an actual bug, but refining the error handling behavior is more of a commitment than I want to make at the moment.

cburroughs accepted this revision.Aug 14 2014, 4:02 PM
cburroughs edited edge metadata.

For giggles I applied the patch instead of following the test plan and we didn't die on a for loop.

This revision is now accepted and ready to land.Aug 14 2014, 4:02 PM
epriestley closed this revision.Aug 14 2014, 7:13 PM
epriestley updated this revision to Diff 24719.

Closed by commit rP40a4eeec77a9 (authored by @epriestley).

Revision Contents
Changeset List

PathSize
src/
infrastructure/
daemon/
bot/
handler/
5 lines

Diff 24719

View Options

src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php

Loading...
Phabricator · Built by Phacility