Improves error messages when fetch fails due to TLS errors.#3116
Conversation
|
@kentonv since you've had thoughts when this was modified previously in #2111 (comment). I wonder if you have any concerns about this change. Could we even maybe go further and expose all exception messages to the user? |
|
Yeah this is a problem. KJ errors should absolutely should not be indiscriminately exposed to applications. At best, they are unlikely to be formatted appropriately; they usually contain a semicolon-delimited list of debug context which would look very strange in a JavaScript error. At worst, they may contain information which is inappropriate to reveal, like internal IDs or something. I apparently failed to chase @jasnell on this last time but this code needs to change back to hide the error. What we usually do instead is instrument the lower-level systems with some way to provide an exception callback which can format exceptions more appropriately. E.g. the HTTP API has |
When running
wrangler dev(or running workerd locally directly), it is possible forfetchto fail with an "internal error". This can likely happen for many reasons, but in my case it was because I was using WARP and had misconfigured TLS certs. The error I was running into was this one: https://github.com/capnproto/capnproto/blob/6e071e34d88a8fc489638535899cd9d02e55bf76/c%2B%2B/src/kj/compat/tls.c%2B%2B#L221.I noticed that we already have some custom logic to catch certain exceptions raised from kj and report them back to the user in a more friendly manner. What I'm curious about is why we don't simply report all errors. I think that matching on strings here is fragile, so I went for a bit of a compromise and changed the code to report any error that occurred in kj. But perhaps that will expose far more than we want.
Test Plan
LLVM_SYMBOLIZER=llvm-symbolizer-14 bazel run --config=release @workerd//src/workerd/server:workerd -- serve $PWD/deps/workerd/samples/pyodide/config.capnp --experimental