Skip to content

Commit 5ebfec1

Browse files
Catherine Gasniermeta-codesync[bot]
authored andcommitted
Classify undeclared exceptions for ServiceRouter
Summary: `glean.glass`'s ServiceRouter server-fatal SLI is ~99.9% undeclared `ApplicationException`, and ~94% of those are downstream `glean.query.prod` failures leaking out of the Glass handler untyped. Two consequences: 1. The SLI measures Glean's throttling, not Glass's availability. The `glean.glass fatals` detector fires on "a Glass client exceeded its Glean per-client QPS share" — something Glass oncall cannot act on. 2. **ServiceRouter marks down healthy Glass hosts.** Undeclared exceptions get `kUndeclaredAppErrorConfig`, which is `logFatal = true` *and* `markdown = true`, so a Glean-side throttle shrinks the routable Glass pool exactly when load is highest. For a Thrift error, the following headers are appended to the response: * 'uex': a short string for the exception type, e.g. ApplicationException * 'ex': the error code for that exception type * 'uexw': the complete error message. hsthrift cannot distinguish undeclared exceptions on its own. The generated `respWriter'` replaces the escaping exception with a fresh `Thrift.ApplicationException` before `handlerWrapper` takes `show (typeOf ex)`, so `uex` is uniformly `ApplicationException` and all triage has to string-match `uexw`. This diff adds function `Glean.Glass.ErrorClassification.srErrorName`, which maps an escaping exception to the `uex` name ServiceRouter should classify it under. D116930502 in configerator then defines the behaviour for each new `uex` value. The mapping of escapting exceptions is as follows: | escaping exception | new `uex` name | |---|---| | downstream `THROTTLING_CLIENT_ID_REQUEST` | `GleanClientThrottled` | | downstream `THROTTLING_ERROR` | `GleanServiceThrottled` | | downstream `APP_QUEUE_TIMEOUT` | `GleanQueueTimeout` | | downstream `HOST_OVERLOAD` / `APP_OVERLOAD` | `GleanOverload` | | GHC allocation limit | `GlassAllocationLimit` | | Glass's own 30s `withTimeout` | `GlassTimeout` | `assignHeaders` emits the new `uex` header. The downstream reason is recovered by matching the `(REASON)` token that `TServiceRouterException::getExceptionMsg` puts at the front of its message. String matching is unavoidable today — `HsChannel.h` stringifies the whole `folly::exception_wrapper` into a `ChannelException Text` and drops the response headers — but only the enum name is matched, never the prose after it, and an unrecognised reason returns `Nothing`. This diff is inert on its own: ServiceRouter falls back to `kUndeclaredAppErrorConfig` for any name with no `appErrorsMap` entry, so behaviour is unchanged until the companion `configerator` change adds the per-name policy. `ex` and `uexw` are deliberately left alone, so the wire-level record still says "undeclared application exception" and no client sees a different response. Reviewed By: bochko Differential Revision: D116930531 fbshipit-source-id: 9f92d799e9d135d8f73379735244c11ca5e15421
1 parent 063b097 commit 5ebfec1

1 file changed

Lines changed: 2 additions & 0 deletions

File tree

lib/Thrift/Channel.hs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ data Response = Response
7474
, respHeader :: Header
7575
}
7676

77+
-- | Thrift transport (THeader) name/value pairs, e.g. @ex@, @uex@, @uexw@.
78+
-- Both name and value are raw bytes rather than text.
7779
type Header = [(ByteString, ByteString)]
7880

7981
newtype ChannelException = ChannelException Text

0 commit comments

Comments
 (0)