[webkit-changes] [WebKit/WebKit] 6ad266: Always log internal loader errors to stderr

[Michael Catanzaro]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 6ad266d724e52b52c46da253141d58173559079f
      https://github.com/WebKit/WebKit/commit/6ad266d724e52b52c46da253141d58173559079f
  Author: Michael Catanzaro <mcatanzaro at redhat.com>
  Date:   2024-05-14 (Tue, 14 May 2024)

  Changed paths:
    M Source/WTF/wtf/Assertions.cpp
    M Source/WTF/wtf/Assertions.h
    M Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp
    M Source/WebCore/platform/network/ResourceErrorBase.cpp
    M Source/WebCore/platform/network/ResourceErrorBase.h

  Log Message:
  -----------
  Always log internal loader errors to stderr
https://bugs.webkit.org/show_bug.cgi?id=273662

Reviewed by Philippe Normand.

Currently when WebKit encounters an internal error during loading, we
release log a stacktrace showing the location of the error. However,
there are several downsides:

 * Release stacktraces are generally very poor quality, so this is
   effectively useless. (On Linux, better backtraces are available if
   built with libbacktrace enabled, but libbacktrace has no releases, so
   I believe no Linux distros do this.)

 * The stacktrace is only logged to the system journal, not to stderr,
   so it's unlikely to actually be noticed.

 * I think it's only logged by default on Linux, because the journald
   implementation of WTFReleaseLogStackTrace ignores whether log
   channels are enabled. (I believe this is a bug in the Linux
   implementation of WTFReleaseLogStackTrace.)

 * Otherwise, release logging has to be enabled manually (using the
   defaults database on macOS, or environment variables on Linux), so
   naturally it won't ever be enabled when needed.

RELEASE_LOG_BACKTRACE is used from only two places:
ResourceErrorBase.cpp, when logging an internal error, and
PixelBufferConformerCV.cpp, which isn't built on Linux. This commit
removes the use of RELEASE_LOG_BACKTRACE from ResourceErrorBase.cpp.
Since there are no other remaining uses of RELEASE_LOG_BACKTRACE, and
since I don't like it, let's move the implementation to
PixelBufferConformerCV.cpp to discourage further use. This allows
simplifying it to assume use of os_log.

WebKit internal loader errors are bugs and worth printing more visibly
so we have a better chance to debug problems, especially sporadic or
unexpected problems that will naturally never occur when we are looking
for them with release logging manually enabled. The backtrace is
probably not really needed here anyway; it's probably generally
sufficient to just log the source file location. Here is what a sample
error message looks like for a fake error that I introduced for test
purposes:

ERROR: WebKit encountered an internal error. This is a WebKit bug.
/home/mcatanzaro/Projects/WebKit/Source/WebKit/WebProcess/gtk/WebProcessMainGtk.cpp(75) : virtual bool WebKit::WebProcessMainGtk::platformInitialize()

In contrast, the release backtrace that I found in my system journal
after encountering an internal error yesterday only tells me that the
problem lies somewhere in libwebkitgtk-6.0.so.4, which is not useful.

* Source/WTF/wtf/Assertions.cpp:
* Source/WTF/wtf/Assertions.h:
* Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp:
(WebCore::logStackTrace):
* Source/WebCore/platform/network/ResourceErrorBase.cpp:
(WebCore::createInternalError):
(WebCore::internalError): Deleted.
* Source/WebCore/platform/network/ResourceErrorBase.h:

Canonical link: https://commits.webkit.org/278778@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications