I use a rock.

Things I've figured out:

I haven't used heat-set inserts yet and I'm not sure if they're trustworthy or not.

Bobbin Case

Found two more quick issues

• Adding search results to dashboard hit an error
• Viewing the prototype calendar event failed after creating an event

Looking at the rest of the UT failures I think those are due to updated/newer versions of linters emitting slightly different results.

Fix new exception message to be phtranslated

Correct the check for an empty username

Revert addition of custom error-types

One actual logical issue inline.

Fine as-is, but see inlines about Conduit error behavior.

One more simplification

Some simplifications

This version looks correct to me in all non-pathological cases, thanks!

I tried creating, updating, and landing a diff revision with the change to throw an exception and none of the workflows were interrupted/error'd.

Switch to using coalesce(), revert json parsing behavior change and throw exception instead, update .editorconfig

Maybe better querystring parsing

Clean up loop logic

Remove calls to phlog

PhutilJSONParserException is sometimes handled specifically:

My preferred behavior is that the error is reported to the user, but not sent to the error log.

If you want to try to handle these cases, I think it would be reasonable to make AphrontApplicationConfiguration store the raw strings somewhere before it sends them to PhutilQueryStringParser so you don't have to copy/paste 100 lines out of readHTTPPOSTData().

But presumably we're more in "fix urlencode() and call it a day" territory here than "rewrite the whole stack to support pure passthrough for a case that probably never occurs in the wild".

I will take the fix urlencode route for now but this is good to know

In D21864#279131, @epriestley wrote:

The two phlog() commands introduced in error handling seem unrelated, can we strip those out of this change? Everything else looks good.

This is a big mess, but here's some general guidance (see also inline):

I think the JSON change is suspicious, everything else looks good.

The two phlog() commands introduced in error handling seem unrelated, can we strip those out of this change? Everything else looks good.

I probably have missed some code paths but this revision has touched a lot of files as-is

Updating to verify before landing that I haven't included further changes

I didn't catch anything that looks suspicious or hazardous. Thanks!

Revert change that doesn't distinguish between invalid vs. inaccessible dashboards

Fixed another issue with user log entries