Page MenuHomePhabricator

Don't log Conduit 404s as errors
ClosedPublic

Authored by joshuaspence on Jul 24 2014, 10:37 PM.
Tags
None
Referenced Files
F13125942: D10042.diff
Tue, Apr 30, 1:25 PM
Unknown Object (File)
Mon, Apr 29, 2:41 PM
Unknown Object (File)
Wed, Apr 24, 10:43 PM
Unknown Object (File)
Thu, Apr 18, 11:18 PM
Unknown Object (File)
Fri, Apr 5, 2:22 PM
Unknown Object (File)
Feb 24 2024, 5:11 PM
Unknown Object (File)
Feb 21 2024, 11:32 PM
Unknown Object (File)
Feb 14 2024, 9:34 AM
Subscribers

Details

Summary

Fixes T5695. A Conduit "method does not exist" exception is somewhat expected... there is no need to phlog the exception.

Test Plan

Called a non-existent Conduit method. Saw no exceptions in the error logs.

Diff Detail

Repository
rP Phabricator
Branch
conduit404
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1891
Build 1892: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Don't log Conduit 404s as errors.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jul 24 2014, 10:50 PM

So you're with fine the @concrete-extensible stuff?

Seems fine to me, generally.

In the next iteration of Conduit, I'd like to push all the wacky error handling more heavily into exceptions, too. For example, we have like 20 methods that define some custom version of "you're trying to update an object which does not exist", and all of that should just be returning something like a 404 response (and maybe even just a 404 response literally).

Yeah. We can rebalance the class tree later (e.g., have an abstract base and then a bunch of final subclasses) if necessary, but it's reasonable to put that off until the next significant iteration since it will probably muck with things anyway.

(And I don't think there's anything inherently bad about a tree like this.)

joshuaspence updated this revision to Diff 24159.

Closed by commit rPbff217efd314 (authored by @joshuaspence).