FLEDGE: Basic diagnostics in devtools console

Basically all steps of the auction process can chose to fill in an
Optional<string>, then AuctionRunner collects them, and
AdAuctionServiceImpl delivers them to a devtools hook that directly
broadcasts them to devtools frontend, bypassing the usual path through
the renderer since the messages may contain sensitive cross-origin
information.

(Also fixes a tiny bug involving double invocation of SendReportTo)

Bug: 1186444
Change-Id: If0bf0c7529b00c4da9f88fcdc467ddc959ba291c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2845592
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879830}

content/browser/devtools/devtools_instrumentation.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 #include "content/browser/devtools/devtools_instrumentation.h"
 
+#include "base/strings/strcat.h"
 #include "base/strings/stringprintf.h"
 #include "components/download/public/common/download_create_info.h"
 #include "components/download/public/common/download_item.h"
@@ -982,6 +983,21 @@ void OnWebTransportHandshakeFailed(
   DispatchToAgents(ftn, &protocol::LogHandler::EntryAdded, entry.get());
 }
 
+void LogWorkletError(RenderFrameHostImpl* frame_host,
+                     const std::string& error) {
+  FrameTreeNode* ftn = frame_host->frame_tree_node();
+  if (!ftn)
+    return;
+  std::string text = base::StrCat({"Worklet error: ", error});
+  auto entry = protocol::Log::LogEntry::Create()
+                   .SetSource(protocol::Log::LogEntry::SourceEnum::Other)
+                   .SetLevel(protocol::Log::LogEntry::LevelEnum::Error)
+                   .SetText(text)
+                   .SetTimestamp(base::Time::Now().ToDoubleT() * 1000.0)
+                   .Build();
+  DispatchToAgents(ftn, &protocol::LogHandler::EntryAdded, entry.get());
+}
+
 void ApplyNetworkContextParamsOverrides(
     BrowserContext* browser_context,
     network::mojom::NetworkContextParams* context_params) {

content/browser/devtools/devtools_instrumentation.h

@@ -211,6 +211,9 @@ void OnWebTransportHandshakeFailed(
     const GURL& url,
     const base::Optional<net::QuicTransportError>& error);
 
+// Adds a debug error message from a worklet to the devtools console.
+void LogWorkletError(RenderFrameHostImpl* frame_host, const std::string& error);
+
 void ApplyNetworkContextParamsOverrides(
     BrowserContext* browser_context,
     network::mojom::NetworkContextParams* network_context_params);

content/browser/interest_group/ad_auction_service_impl.cc

@@ -13,6 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/strings/stringprintf.h"
+#include "content/browser/devtools/devtools_instrumentation.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/service_sandbox_type.h"
 #include "content/browser/storage_partition_impl.h"
@@ -322,10 +323,17 @@ void AdAuctionServiceImpl::WorkletComplete(
     const url::Origin& owner,
     const std::string& name,
     auction_worklet::mojom::WinningBidderReportPtr bidder_report,
-    auction_worklet::mojom::SellerReportPtr seller_report) {
+    auction_worklet::mojom::SellerReportPtr seller_report,
+    const std::vector<std::string>& errors) {
   // Release process if needed.
   AuctionComplete();
 
+  // Forward debug information to devtools.
+  for (const std::string& error : errors) {
+    devtools_instrumentation::LogWorkletError(
+        static_cast<RenderFrameHostImpl*>(render_frame_host()), error);
+  }
+
   // Check if returned winner's information is valid.
   ValidatedResult result =
       ValidateAuctionResult(bidders, render_url, owner, name);

content/browser/interest_group/ad_auction_service_impl.h

@@ -94,7 +94,8 @@ class CONTENT_EXPORT AdAuctionServiceImpl final
       const url::Origin& owner,
       const std::string& name,
       auction_worklet::mojom::WinningBidderReportPtr bidder_report,
-      auction_worklet::mojom::SellerReportPtr seller_report);
+      auction_worklet::mojom::SellerReportPtr seller_report,
+      const std::vector<std::string>& errors);
 
   // Returns an untrusted URLLoaderFactory created by the RenderFrameHost,
   // suitable for loading URLs like subresources. Caches the factory in

content/services/auction_worklet/auction_downloader.cc

@@ -11,7 +11,10 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "net/base/net_errors.h"
 #include "net/http/http_request_headers.h"
+#include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/redirect_info.h"
 #include "services/network/public/cpp/resource_request.h"
@@ -87,7 +90,8 @@ AuctionDownloader::AuctionDownloader(
     const GURL& source_url,
     MimeType mime_type,
     AuctionDownloaderCallback auction_downloader_callback)
-    : mime_type_(mime_type),
+    : source_url_(source_url),
+      mime_type_(mime_type),
       auction_downloader_callback_(std::move(auction_downloader_callback)) {
   DCHECK(auction_downloader_callback_);
   auto resource_request = std::make_unique<network::ResourceRequest>();
@@ -100,8 +104,6 @@ AuctionDownloader::AuctionDownloader(
   simple_url_loader_ = network::SimpleURLLoader::Create(
       std::move(resource_request), kTrafficAnnotation);
 
-  // TODO(mmenke): Reject unexpected charsets.
-
   // Abort on redirects.
   // TODO(mmenke): May want a browser-side proxy to block redirects instead.
   simple_url_loader_->SetOnRedirectCallback(base::BindRepeating(
@@ -120,19 +122,54 @@ void AuctionDownloader::OnBodyReceived(std::unique_ptr<std::string> body) {
 
   auto simple_url_loader = std::move(simple_url_loader_);
   std::string allow_fledge;
-  if (!body || !simple_url_loader->ResponseInfo()->headers ||
-      !simple_url_loader->ResponseInfo()->headers->GetNormalizedHeader(
-          "X-Allow-FLEDGE", &allow_fledge) ||
-      !base::EqualsCaseInsensitiveASCII(allow_fledge, "true") ||
-      // Note that ResponseInfo's `mime_type` is always lowercase.
-      !MimeTypeIsConsistent(mime_type_,
-                            simple_url_loader->ResponseInfo()->mime_type) ||
-      !IsAllowedCharset(simple_url_loader->ResponseInfo()->charset, *body)) {
-    std::move(auction_downloader_callback_).Run(nullptr);
-    return;
-  }
 
-  std::move(auction_downloader_callback_).Run(std::move(body));
+  if (!body) {
+    std::string error_msg;
+    if (simple_url_loader->ResponseInfo() &&
+        simple_url_loader->ResponseInfo()->headers &&
+        simple_url_loader->ResponseInfo()->headers->response_code() / 100 !=
+            2) {
+      int status = simple_url_loader->ResponseInfo()->headers->response_code();
+      error_msg = base::StringPrintf(
+          "Failed to load %s HTTP status = %d %s.", source_url_.spec().c_str(),
+          status,
+          simple_url_loader->ResponseInfo()->headers->GetStatusText().c_str());
+    } else {
+      error_msg = base::StringPrintf(
+          "Failed to load %s error = %s.", source_url_.spec().c_str(),
+          net::ErrorToString(simple_url_loader->NetError()).c_str());
+    }
+    std::move(auction_downloader_callback_).Run(nullptr /* body */, error_msg);
+  } else if (!simple_url_loader->ResponseInfo()->headers ||
+             !simple_url_loader->ResponseInfo()->headers->GetNormalizedHeader(
+                 "X-Allow-FLEDGE", &allow_fledge) ||
+             !base::EqualsCaseInsensitiveASCII(allow_fledge, "true")) {
+    std::move(auction_downloader_callback_)
+        .Run(nullptr /* body */,
+             base::StringPrintf(
+                 "Rejecting load of %s due to lack of X-Allow-FLEDGE: true.",
+                 source_url_.spec().c_str()));
+  } else if (!MimeTypeIsConsistent(
+                 mime_type_,
+                 // ResponseInfo's `mime_type` is always lowercase.
+                 simple_url_loader->ResponseInfo()->mime_type)) {
+    std::move(auction_downloader_callback_)
+        .Run(nullptr /* body */,
+             base::StringPrintf(
+                 "Rejecting load of %s due to unexpected MIME type.",
+                 source_url_.spec().c_str()));
+  } else if (!IsAllowedCharset(simple_url_loader->ResponseInfo()->charset,
+                               *body)) {
+    std::move(auction_downloader_callback_)
+        .Run(nullptr /* body */,
+             base::StringPrintf(
+                 "Rejecting load of %s due to unexpected charset.",
+                 source_url_.spec().c_str()));
+  } else {
+    // All OK!
+    std::move(auction_downloader_callback_)
+        .Run(std::move(body), base::nullopt /* error_msg */);
+  }
 }
 
 void AuctionDownloader::OnRedirect(
@@ -144,7 +181,9 @@ void AuctionDownloader::OnRedirect(
   // Need to cancel the load, to prevent the request from continuing.
   simple_url_loader_.reset();
 
-  std::move(auction_downloader_callback_).Run(nullptr /* body */);
+  std::move(auction_downloader_callback_)
+      .Run(nullptr /* body */, base::StringPrintf("Unexpected redirect on %s.",
+                                                  source_url_.spec().c_str()));
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/auction_downloader.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/optional.h"
 #include "net/url_request/redirect_info.h"
 #include "services/network/public/mojom/url_loader_factory.mojom-forward.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
@@ -32,11 +33,9 @@ class AuctionDownloader {
   };
 
   // Passes in nullptr on failure. Always invoked asynchronously.
-  //
-  // TODO(mmenke): Pass along some sort of error string on failure, to make
-  // debugging easier.
   using AuctionDownloaderCallback =
-      base::OnceCallback<void(std::unique_ptr<std::string> response_body)>;
+      base::OnceCallback<void(std::unique_ptr<std::string> response_body,
+                              base::Optional<std::string> error)>;
 
   // Starts loading the worklet script on construction. Callback will be invoked
   // asynchronously once the data has been fetched or an error has occurred.
@@ -55,6 +54,7 @@ class AuctionDownloader {
                   const network::mojom::URLResponseHead& response_head,
                   std::vector<std::string>* removed_headers);
 
+  const GURL source_url_;
   const MimeType mime_type_;
   std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
   AuctionDownloaderCallback auction_downloader_callback_;

content/services/auction_worklet/auction_downloader_unittest.cc

@@ -52,23 +52,32 @@ class AuctionDownloaderTest : public testing::Test {
     return std::move(body_);
   }
 
+  // Helper to avoid checking has_value all over the place.
+  std::string last_error_msg() const {
+    return error_.value_or("Not an error.");
+  }
+
  protected:
-  void DownloadCompleteCallback(std::unique_ptr<std::string> body) {
+  void DownloadCompleteCallback(std::unique_ptr<std::string> body,
+                                base::Optional<std::string> error) {
     DCHECK(!body_);
     DCHECK(run_loop_);
     body_ = std::move(body);
+    error_ = std::move(error);
+    EXPECT_EQ(error_.has_value(), !body_);
     run_loop_->Quit();
   }
 
   base::test::TaskEnvironment task_environment_;
 
-  const GURL url_ = GURL("https://url.test/");
+  const GURL url_ = GURL("https://url.test/script.js");
 
   AuctionDownloader::MimeType mime_type_ =
       AuctionDownloader::MimeType::kJavascript;
 
   std::unique_ptr<base::RunLoop> run_loop_;
   std::unique_ptr<std::string> body_;
+  base::Optional<std::string> error_;
 
   network::TestURLLoaderFactory url_loader_factory_;
 };
@@ -79,6 +88,9 @@ TEST_F(AuctionDownloaderTest, NetworkError) {
   url_loader_factory_.AddResponse(url_, nullptr /* head */, kAsciiResponseBody,
                                   status);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Failed to load https://url.test/script.js error = net::ERR_FAILED.",
+      last_error_msg());
 }
 
 // HTTP 404 responses are trested as failures.
@@ -88,6 +100,9 @@ TEST_F(AuctionDownloaderTest, HttpError) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, kAllowFledgeHeader, net::HTTP_NOT_FOUND);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Failed to load https://url.test/script.js HTTP status = 404 Not Found.",
+      last_error_msg());
 }
 
 TEST_F(AuctionDownloaderTest, AllowFledge) {
@@ -102,26 +117,50 @@ TEST_F(AuctionDownloaderTest, AllowFledge) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, "X-Allow-FLEDGE: false");
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, "X-Allow-FLEDGE: sometimes");
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, "X-Allow-FLEDGE: ");
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, "X-Allow-Hats: true");
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, "");
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody, base::nullopt);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to lack of "
+      "X-Allow-FLEDGE: true.",
+      last_error_msg());
 }
 
 // Redirect responses are treated as failures.
@@ -140,6 +179,8 @@ TEST_F(AuctionDownloaderTest, Redirect) {
               kAsciiResponseBody, kAllowFledgeHeader, net::HTTP_OK,
               std::move(redirects));
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ("Unexpected redirect on https://url.test/script.js.",
+            last_error_msg());
 }
 
 TEST_F(AuctionDownloaderTest, Success) {
@@ -156,40 +197,72 @@ TEST_F(AuctionDownloaderTest, MimeType) {
   AddResponse(&url_loader_factory_, url_, kJsonMimeType, kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // Javascript request, no response type.
   AddResponse(&url_loader_factory_, url_, base::nullopt, kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // Javascript request, empty response type.
   AddResponse(&url_loader_factory_, url_, "", kUtf8Charset, kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // Javascript request, unknown response type.
   AddResponse(&url_loader_factory_, url_, "blobfish", kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // JSON request, Javascript response type.
   mime_type_ = AuctionDownloader::MimeType::kJson;
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // JSON request, no response type.
   AddResponse(&url_loader_factory_, url_, base::nullopt, kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // JSON request, empty response type.
   AddResponse(&url_loader_factory_, url_, "", kUtf8Charset, kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // JSON request, unknown response type.
   AddResponse(&url_loader_factory_, url_, "blobfish", kUtf8Charset,
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected MIME "
+      "type.",
+      last_error_msg());
 
   // JSON request, JSON response type.
   mime_type_ = AuctionDownloader::MimeType::kJson;
@@ -241,6 +314,10 @@ TEST_F(AuctionDownloaderTest, MimeTypeVariants) {
     AddResponse(&url_loader_factory_, url_, javascript_type, kUtf8Charset,
                 kAsciiResponseBody);
     EXPECT_FALSE(RunRequest());
+    EXPECT_EQ(
+        "Rejecting load of https://url.test/script.js due to unexpected MIME "
+        "type.",
+        last_error_msg());
   }
 
   for (const char* json_type : kJsonMimeTypes) {
@@ -248,6 +325,10 @@ TEST_F(AuctionDownloaderTest, MimeTypeVariants) {
     AddResponse(&url_loader_factory_, url_, json_type, kUtf8Charset,
                 kAsciiResponseBody);
     EXPECT_FALSE(RunRequest());
+    EXPECT_EQ(
+        "Rejecting load of https://url.test/script.js due to unexpected MIME "
+        "type.",
+        last_error_msg());
 
     mime_type_ = AuctionDownloader::MimeType::kJson;
     AddResponse(&url_loader_factory_, url_, json_type, kUtf8Charset,
@@ -259,18 +340,31 @@ TEST_F(AuctionDownloaderTest, MimeTypeVariants) {
 }
 
 TEST_F(AuctionDownloaderTest, Charset) {
+  mime_type_ = AuctionDownloader::MimeType::kJson;
   // Unknown/unsupported charsets should result in failure.
   AddResponse(&url_loader_factory_, url_, kJsonMimeType, "fred",
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
+
   AddResponse(&url_loader_factory_, url_, kJsonMimeType, "iso-8859-1",
               kAsciiResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
 
   // ASCII charset should restrict response bodies to ASCII characters.
+  mime_type_ = AuctionDownloader::MimeType::kJavascript;
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kAsciiCharset,
               kUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
+
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kAsciiCharset,
               kAsciiResponseBody);
   std::unique_ptr<std::string> body = RunRequest();
@@ -279,9 +373,16 @@ TEST_F(AuctionDownloaderTest, Charset) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kAsciiCharset,
               kUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
+
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kAsciiCharset,
               kNonUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
 
   // UTF-8 charset should restrict response bodies to valid UTF-8 characters.
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
@@ -297,6 +398,9 @@ TEST_F(AuctionDownloaderTest, Charset) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, kUtf8Charset,
               kNonUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
 
   // Null charset should act like UTF-8.
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, base::nullopt,
@@ -312,6 +416,9 @@ TEST_F(AuctionDownloaderTest, Charset) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, base::nullopt,
               kNonUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
 
   // Empty charset should act like UTF-8.
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, "",
@@ -327,6 +434,9 @@ TEST_F(AuctionDownloaderTest, Charset) {
   AddResponse(&url_loader_factory_, url_, kJavascriptMimeType, "",
               kNonUtf8ResponseBody);
   EXPECT_FALSE(RunRequest());
+  EXPECT_EQ(
+      "Rejecting load of https://url.test/script.js due to unexpected charset.",
+      last_error_msg());
 }
 
 }  // namespace

content/services/auction_worklet/auction_runner.cc

@@ -80,7 +80,7 @@ void AuctionRunner::StartBidding() {
               base::BindOnce(&AuctionRunner::OnTrustedSignalsLoaded,
                              base::Unretained(this), bid_state));
     } else {
-      OnTrustedSignalsLoaded(bid_state, false);
+      OnTrustedSignalsLoaded(bid_state, false, base::nullopt);
     }
   }
 
@@ -92,9 +92,16 @@ void AuctionRunner::StartBidding() {
                      base::Unretained(this)));
 }
 
-void AuctionRunner::OnBidderScriptLoaded(BidState* state, bool load_result) {
+void AuctionRunner::OnBidderScriptLoaded(
+    BidState* state,
+    bool load_result,
+    base::Optional<std::string> error_msg) {
   DCHECK(!state->bidder_script_loaded);
   DCHECK(!state->failed);
+
+  if (error_msg.has_value())
+    errors_.push_back(std::move(error_msg).value());
+
   state->bidder_script_loaded = true;
   if (!load_result) {
     state->failed = true;
@@ -108,8 +115,15 @@ void AuctionRunner::OnBidderScriptLoaded(BidState* state, bool load_result) {
   MaybeRunBid(state);
 }
 
-void AuctionRunner::OnTrustedSignalsLoaded(BidState* state, bool load_result) {
+void AuctionRunner::OnTrustedSignalsLoaded(
+    BidState* state,
+    bool load_result,
+    base::Optional<std::string> error_msg) {
   DCHECK(!state->trusted_signals_loaded);
+
+  if (error_msg.has_value())
+    errors_.push_back(std::move(error_msg).value());
+
   state->trusted_signals_loaded = true;
   if (!load_result)
     state->trusted_bidding_signals.reset();
@@ -133,10 +147,17 @@ void AuctionRunner::RunBid(BidState* state) {
   base::TimeTicks start = base::TimeTicks::Now();
   state->bid_result =
       state->bidder_worklet->GenerateBid(state->trusted_bidding_signals.get());
+  if (state->bid_result.error_msg.has_value())
+    errors_.push_back(std::move(state->bid_result.error_msg).value());
   state->bid_duration = base::TimeTicks::Now() - start;
 }
 
-void AuctionRunner::OnSellerWorkletLoaded(bool load_result) {
+void AuctionRunner::OnSellerWorkletLoaded(
+    bool load_result,
+    base::Optional<std::string> error_msg) {
+  if (error_msg.has_value())
+    errors_.push_back(std::move(error_msg).value());
+
   if (load_result) {
     seller_loaded_ = true;
     if (ReadyToScore())
@@ -177,10 +198,13 @@ void AuctionRunner::ScoreOne() {
 }
 
 SellerWorklet::ScoreResult AuctionRunner::ScoreBid(const BidState* state) {
-  return seller_worklet_->ScoreAd(
+  SellerWorklet::ScoreResult result = seller_worklet_->ScoreAd(
       state->bid_result.ad, state->bid_result.bid, *auction_config_,
       browser_signals_->top_frame_origin.host(), state->bidder->group->owner,
       AdRenderFingerprint(state), state->bid_duration);
+  if (result.error_msg.has_value())
+    errors_.push_back(std::move(result.error_msg).value());
+  return result;
 }
 
 std::string AuctionRunner::AdRenderFingerprint(const BidState* state) {
@@ -225,26 +249,32 @@ void AuctionRunner::CompleteAuction() {
 
 SellerWorklet::Report AuctionRunner::ReportSellerResult(
     const BidState* best_bid) {
-  return seller_worklet_->ReportResult(
+  SellerWorklet::Report result = seller_worklet_->ReportResult(
       *auction_config_, browser_signals_->top_frame_origin.host(),
       best_bid->bidder->group->owner, best_bid->bid_result.render_url,
       AdRenderFingerprint(best_bid), best_bid->bid_result.bid,
       best_bid->score_result.score);
+  if (result.error_msg.has_value())
+    errors_.push_back(std::move(result.error_msg).value());
+  return result;
 }
 
 BidderWorklet::ReportWinResult AuctionRunner::ReportBidWin(
     const BidState* best_bid,
     const SellerWorklet::Report& seller_report) {
-  return best_bid->bidder_worklet->ReportWin(
+  BidderWorklet::ReportWinResult result = best_bid->bidder_worklet->ReportWin(
       seller_report.signals_for_winner, best_bid->bid_result.render_url,
       AdRenderFingerprint(best_bid), best_bid->bid_result.bid);
+  if (result.error_msg.has_value())
+    errors_.push_back(std::move(result.error_msg).value());
+  return result;
 }
 
 void AuctionRunner::FailAuction() {
   std::move(callback_).Run(
       GURL(), url::Origin(), std::string(),
       mojom::WinningBidderReport::New(false /* success */, GURL()),
-      mojom::SellerReport::New(false /* success */, "", GURL()));
+      mojom::SellerReport::New(false /* success */, "", GURL()), errors_);
   delete this;
 }
 
@@ -259,7 +289,8 @@ void AuctionRunner::ReportSuccess(
                                       bidder_report.report_url),
       mojom::SellerReport::New(seller_report.success,
                                seller_report.signals_for_winner,
-                               seller_report.report_url));
+                               seller_report.report_url),
+      errors_);
   delete this;
 }
 

content/services/auction_worklet/auction_runner.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_SERVICES_AUCTION_WORKLET_AUCTION_RUNNER_H_
 #define CONTENT_SERVICES_AUCTION_WORKLET_AUCTION_RUNNER_H_
 
+#include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
@@ -85,14 +86,19 @@ class AuctionRunner {
   ~AuctionRunner();
 
   void StartBidding();
-  void OnBidderScriptLoaded(BidState* state, bool load_result);
+  void OnBidderScriptLoaded(BidState* state,
-  void OnTrustedSignalsLoaded(BidState* state, bool load_result);
+                            bool load_result,
+                            base::Optional<std::string> error_msg);
+  void OnTrustedSignalsLoaded(BidState* state,
+                              bool load_result,
+                              base::Optional<std::string> error_msg);
   void MaybeRunBid(BidState* state);
   void RunBid(BidState* state);
 
   // True if all bid results and the seller script load are complete.
   bool ReadyToScore() const { return outstanding_bids_ == 0 && seller_loaded_; }
-  void OnSellerWorkletLoaded(bool load_result);
+  void OnSellerWorkletLoaded(bool load_result,
+                             base::Optional<std::string> error_msg);
 
   // Lets the seller score a single outstanding bid, if any, and then either
   // re-queues itself on event loop if there is more to check, or proceeds to
@@ -145,6 +151,9 @@ class AuctionRunner {
   // that can be done.
   bool seller_loaded_ = false;
   size_t seller_considering_ = 0;
+
+  // All errors reported by worklets thus far.
+  std::vector<std::string> errors_;
 };
 
 }  // namespace auction_worklet

content/services/auction_worklet/auction_runner_unittest.cc

@@ -14,6 +14,7 @@
 #include "content/services/auction_worklet/worklet_test_util.h"
 #include "net/http/http_status_code.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace auction_worklet {
@@ -205,6 +206,7 @@ class AuctionRunnerTest : public testing::Test {
     std::string interest_group_name;
     mojom::WinningBidderReportPtr bidder_report;
     mojom::SellerReportPtr seller_report;
+    std::vector<std::string> errors;
   };
 
   Result RunAuctionAndWait(const GURL& seller_decision_logic_url,
@@ -238,12 +240,14 @@ class AuctionRunnerTest : public testing::Test {
                                        const url::Origin& interest_group_owner,
                                        const std::string& interest_group_name,
                                        mojom::WinningBidderReportPtr bid_report,
-                                       mojom::SellerReportPtr seller_report) {
+                                       mojom::SellerReportPtr seller_report,
+                                       const std::vector<std::string>& errors) {
           result.ad_url = ad_url;
           result.interest_group_owner = interest_group_owner;
           result.interest_group_name = interest_group_name;
           result.bidder_report = std::move(bid_report);
           result.seller_report = std::move(seller_report);
+          result.errors = errors;
           run_loop.Quit();
         }));
     run_loop.Run();
@@ -346,6 +350,7 @@ TEST_F(AuctionRunnerTest, Basic) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(res.errors, testing::ElementsAre());
 }
 
 // An auction where one bid is successful, another's script 404s.
@@ -381,6 +386,10 @@ TEST_F(AuctionRunnerTest, OneBidOne404) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(
+      res.errors,
+      testing::ElementsAre("Failed to load https://anotheradthing.com/bids.js "
+                           "HTTP status = 404 Not Found."));
 }
 
 // An auction where one bid is successful, another's script does not provide a
@@ -419,6 +428,9 @@ TEST_F(AuctionRunnerTest, OneBidOneNotMade) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(res.errors,
+              testing::ElementsAre("https://anotheradthing.com/bids.js "
+                                   "`generateBid` is not a function."));
 }
 
 // An auction where no bidding scripts load successfully.
@@ -444,6 +456,12 @@ TEST_F(AuctionRunnerTest, NoBids) {
   EXPECT_EQ("", res.seller_report->signals_for_winner_json);
   EXPECT_FALSE(res.bidder_report->report_requested);
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
+  EXPECT_THAT(
+      res.errors,
+      testing::ElementsAre("Failed to load https://adplatform.com/offers.js "
+                           "HTTP status = 404 Not Found.",
+                           "Failed to load https://anotheradthing.com/bids.js "
+                           "HTTP status = 404 Not Found."));
 }
 
 // An auction where none of the bidding scripts has a valid bidding function.
@@ -470,6 +488,12 @@ TEST_F(AuctionRunnerTest, NoBidMadeByScript) {
   EXPECT_EQ("", res.seller_report->signals_for_winner_json);
   EXPECT_FALSE(res.bidder_report->report_requested);
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
+  EXPECT_THAT(
+      res.errors,
+      testing::ElementsAre(
+          "https://adplatform.com/offers.js `generateBid` is not a function.",
+          "https://anotheradthing.com/bids.js `generateBid` is not a "
+          "function."));
 }
 
 // An auction where the seller script doesn't have a scoring function.
@@ -503,6 +527,11 @@ TEST_F(AuctionRunnerTest, SellerRejectsAll) {
   EXPECT_EQ("", res.seller_report->signals_for_winner_json);
   EXPECT_FALSE(res.bidder_report->report_requested);
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
+  EXPECT_THAT(res.errors,
+              testing::ElementsAre("https://adstuff.publisher1.com/auction.js "
+                                   "`scoreAd` is not a function.",
+                                   "https://adstuff.publisher1.com/auction.js "
+                                   "`scoreAd` is not a function."));
 }
 
 // An auction where seller rejects one bid when scoring.
@@ -560,6 +589,10 @@ TEST_F(AuctionRunnerTest, NoSellerScript) {
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
 
   EXPECT_EQ(0, url_loader_factory_.NumPending());
+  EXPECT_THAT(res.errors,
+              testing::ElementsAre(
+                  "Failed to load https://adstuff.publisher1.com/auction.js "
+                  "HTTP status = 404 Not Found."));
 }
 
 // An auction where bidders don't requested trusted bidding signals.
@@ -604,6 +637,7 @@ TEST_F(AuctionRunnerTest, NoTrustedBiddingSignals) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(res.errors, testing::ElementsAre());
 }
 
 // An auction where trusted bidding signals are requested, but the fetch 404s.
@@ -640,6 +674,15 @@ TEST_F(AuctionRunnerTest, TrustedBiddingSignals404) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(res.errors,
+              testing::ElementsAre("Failed to load "
+                                   "https://trustedsignaller.org/"
+                                   "signals?hostname=publisher1.com&keys=k1,k2 "
+                                   "HTTP status = 404 Not Found.",
+                                   "Failed to load "
+                                   "https://trustedsignaller.org/"
+                                   "signals?hostname=publisher1.com&keys=l1,l2 "
+                                   "HTTP status = 404 Not Found."));
 }
 
 // A successful auction where seller reporting worklet doesn't set a URL.
@@ -678,6 +721,7 @@ TEST_F(AuctionRunnerTest, NoReportResultUrl) {
   EXPECT_TRUE(res.bidder_report->report_requested);
   EXPECT_EQ("https://buyer-reporting.example.com/",
             res.bidder_report->report_url.spec());
+  EXPECT_THAT(res.errors, testing::ElementsAre());
 }
 
 // A successful auction where bidder reporting worklet doesn't set a URL.
@@ -717,6 +761,7 @@ TEST_F(AuctionRunnerTest, NoReportWinUrl) {
             res.seller_report->signals_for_winner_json);
   EXPECT_FALSE(res.bidder_report->report_requested);
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
+  EXPECT_THAT(res.errors, testing::ElementsAre());
 }
 
 // A successful auction where neither reporting worklets sets a URL.
@@ -756,6 +801,7 @@ TEST_F(AuctionRunnerTest, NeitherReportUrl) {
             res.seller_report->signals_for_winner_json);
   EXPECT_FALSE(res.bidder_report->report_requested);
   EXPECT_TRUE(res.bidder_report->report_url.is_empty());
+  EXPECT_THAT(res.errors, testing::ElementsAre());
 }
 
 }  // namespace

content/services/auction_worklet/auction_v8_helper.cc

@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/synchronization/lock.h"
@@ -275,7 +276,8 @@ bool AuctionV8Helper::ExtractJson(v8::Local<v8::Context> context,
 
 v8::MaybeLocal<v8::UnboundScript> AuctionV8Helper::Compile(
     const std::string& src,
-    const GURL& src_url) {
+    const GURL& src_url,
+    base::Optional<std::string>& error_out) {
   v8::Isolate* v8_isolate = isolate();
 
   v8::MaybeLocal<v8::String> src_string = CreateUtf8String(src);
@@ -291,8 +293,10 @@ v8::MaybeLocal<v8::UnboundScript> AuctionV8Helper::Compile(
   auto result = v8::ScriptCompiler::CompileUnboundScript(
       v8_isolate, &script_source, v8::ScriptCompiler::kNoCompileOptions,
       v8::ScriptCompiler::NoCacheReason::kNoCacheNoReason);
-  if (try_catch.HasCaught())
+  if (try_catch.HasCaught()) {
-    PrintMessage(v8_isolate->GetCurrentContext(), try_catch.Message());
+    error_out = FormatExceptionMessage(v8_isolate->GetCurrentContext(),
+                                       try_catch.Message());
+  }
   return result;
 }
 
@@ -300,7 +304,8 @@ v8::MaybeLocal<v8::Value> AuctionV8Helper::RunScript(
     v8::Local<v8::Context> context,
     v8::Local<v8::UnboundScript> script,
     base::StringPiece script_name,
-    base::span<v8::Local<v8::Value>> args) {
+    base::span<v8::Local<v8::Value>> args,
+    base::Optional<std::string>& error_out) {
   DCHECK_EQ(isolate(), context->GetIsolate());
 
   v8::Local<v8::String> v8_script_name;
@@ -313,8 +318,15 @@ v8::MaybeLocal<v8::Value> AuctionV8Helper::RunScript(
   v8::TryCatch try_catch(isolate());
   ScriptTimeoutHelper timeout_helper(isolate(), script_timeout_);
   auto result = local_script->Run(context);
+
+  if (try_catch.HasTerminated()) {
+    error_out = base::StrCat({FormatValue(isolate(), script->GetScriptName()),
+                              " top-level execution timed out."});
+    return v8::MaybeLocal<v8::Value>();
+  }
+
   if (try_catch.HasCaught()) {
-    PrintMessage(context, try_catch.Message());
+    error_out = FormatExceptionMessage(context, try_catch.Message());
     return v8::MaybeLocal<v8::Value>();
   }
 
@@ -322,35 +334,47 @@ v8::MaybeLocal<v8::Value> AuctionV8Helper::RunScript(
     return v8::MaybeLocal<v8::Value>();
 
   v8::Local<v8::Value> function;
-  if (!context->Global()->Get(context, v8_script_name).ToLocal(&function))
+  if (!context->Global()->Get(context, v8_script_name).ToLocal(&function)) {
+    error_out = base::StrCat({FormatValue(isolate(), script->GetScriptName()),
+                              " function `", script_name, "` not found."});
     return v8::MaybeLocal<v8::Value>();
+  }
 
-  if (!function->IsFunction())
+  if (!function->IsFunction()) {
+    error_out = base::StrCat({FormatValue(isolate(), script->GetScriptName()),
+                              " `", script_name, "` is not a function."});
     return v8::MaybeLocal<v8::Value>();
+  }
 
   v8::MaybeLocal<v8::Value> func_result = v8::Function::Cast(*function)->Call(
       context, context->Global(), args.size(), args.data());
+  if (try_catch.HasTerminated()) {
+    error_out = base::StrCat({FormatValue(isolate(), script->GetScriptName()),
+                              " execution of `", script_name, "` timed out."});
+    return v8::MaybeLocal<v8::Value>();
+  }
   if (try_catch.HasCaught()) {
-    PrintMessage(context, try_catch.Message());
+    error_out = FormatExceptionMessage(context, try_catch.Message());
     return v8::MaybeLocal<v8::Value>();
   }
   return func_result;
 }
 
 // static
-void AuctionV8Helper::PrintMessage(v8::Local<v8::Context> context,
+std::string AuctionV8Helper::FormatExceptionMessage(
-                                   v8::Local<v8::Message> message) {
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Message> message) {
   if (message.IsEmpty()) {
-    LOG(ERROR) << "Unknown exception";
+    return "Unknown exception.";
   } else {
     v8::Isolate* isolate = message->GetIsolate();
     int line_num;
-    LOG(ERROR) << FormatValue(isolate, message->GetScriptResourceName())
+    return base::StrCat(
-               << (!context.IsEmpty() &&
+        {FormatValue(isolate, message->GetScriptResourceName()),
-                           message->GetLineNumber(context).To(&line_num)
+         !context.IsEmpty() && message->GetLineNumber(context).To(&line_num)
-                       ? std::string(":") + base::NumberToString(line_num)
+             ? std::string(":") + base::NumberToString(line_num)
-                       : std::string())
+             : std::string(),
-               << " " << FormatValue(isolate, message->Get());
+         " ", FormatValue(isolate, message->Get()), "."});
   }
 }
 

content/services/auction_worklet/auction_v8_helper.h

@@ -6,9 +6,11 @@
 #define CONTENT_SERVICES_AUCTION_WORKLET_AUCTION_V8_HELPER_H_
 
 #include <memory>
+#include <string>
 
 #include "base/compiler_specific.h"
 #include "base/containers/span.h"
+#include "base/optional.h"
 #include "base/strings/string_piece.h"
 #include "base/time/time.h"
 #include "gin/public/isolate_holder.h"
@@ -108,9 +110,12 @@ class AuctionV8Helper {
                    std::string* out);
 
   // Compiles the provided script. Despite not being bound to a context, there
-  // still must be an active context for this method to be invoked.
+  // still must be an active context for this method to be invoked. In case of
-  v8::MaybeLocal<v8::UnboundScript> Compile(const std::string& src,
+  // an error, sets `error_out`.
-                                            const GURL& src_url);
+  v8::MaybeLocal<v8::UnboundScript> Compile(
+      const std::string& src,
+      const GURL& src_url,
+      base::Optional<std::string>& error_out);
 
   // Binds a script and runs it in the passed in context, returning the result.
   // Note that the returned value could include references to objects or
@@ -122,19 +127,21 @@ class AuctionV8Helper {
   //
   // Running this multiple times in the same context will re-load the entire
   // script file in the context, and then run the script again.
-  v8::MaybeLocal<v8::Value> RunScript(
+  //
-      v8::Local<v8::Context> context,
+  // In case of an error, sets `error_out`.
-      v8::Local<v8::UnboundScript> script,
+  v8::MaybeLocal<v8::Value> RunScript(v8::Local<v8::Context> context,
-      base::StringPiece script_name,
+                                      v8::Local<v8::UnboundScript> script,
-      base::span<v8::Local<v8::Value>> args = {});
+                                      base::StringPiece script_name,
+                                      base::span<v8::Local<v8::Value>> args,
+                                      base::Optional<std::string>& error_out);
 
   void set_script_timeout_for_testing(base::TimeDelta script_timeout) {
     script_timeout_ = script_timeout;
   }
 
  private:
-  static void PrintMessage(v8::Local<v8::Context> context,
+  static std::string FormatExceptionMessage(v8::Local<v8::Context> context,
-                           v8::Local<v8::Message> message);
+                                            v8::Local<v8::Message> message);
   static std::string FormatValue(v8::Isolate* isolate,
                                  v8::Local<v8::Value> val);
 

content/services/auction_worklet/auction_v8_helper_unittest.cc

@@ -6,13 +6,18 @@
 
 #include <string>
 
+#include "base/optional.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
 #include "gin/converter.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
 
+using testing::HasSubstr;
+using testing::StartsWith;
+
 namespace auction_worklet {
 
 class AuctionV8HelperTest : public testing::Test {
@@ -32,53 +37,77 @@ TEST_F(AuctionV8HelperTest, Basic) {
   v8::Local<v8::UnboundScript> script;
   {
     v8::Context::Scope ctx(helper_.scratch_context());
-    ASSERT_TRUE(
+    base::Optional<std::string> error_msg;
-        helper_
+    ASSERT_TRUE(helper_
-            .Compile("function foo() { return 1;}", GURL("https://foo.test/"))
+                    .Compile("function foo() { return 1;}",
-            .ToLocal(&script));
+                             GURL("https://foo.test/"), error_msg)
+                    .ToLocal(&script));
+    EXPECT_FALSE(error_msg.has_value());
   }
 
   for (v8::Local<v8::Context> context :
        {helper_.scratch_context(), helper_.CreateContext()}) {
+    base::Optional<std::string> error_msg;
     v8::Context::Scope ctx(context);
     v8::Local<v8::Value> result;
-    ASSERT_TRUE(helper_.RunScript(context, script, "foo").ToLocal(&result));
+    ASSERT_TRUE(helper_
+                    .RunScript(context, script, "foo",
+                               base::span<v8::Local<v8::Value>>(), error_msg)
+                    .ToLocal(&result));
     int int_result = 0;
     ASSERT_TRUE(gin::ConvertFromV8(helper_.isolate(), result, &int_result));
     EXPECT_EQ(1, int_result);
+    EXPECT_FALSE(error_msg.has_value());
   }
 }
 
 // Check that timing out scripts works.
 TEST_F(AuctionV8HelperTest, Timeout) {
-  const char* kHangingScripts[] = {
+  struct HangingScript {
+    const char* script;
+    bool top_level_hangs;
+  };
+
+  const HangingScript kHangingScripts[] = {
       // Script that times out when run. Its foo() method returns 1, but should
       // never be called.
-      R"(function foo() { return 1;}
+      {R"(function foo() { return 1;}
-         while(1);)",
+        while(1);)",
+       true},
+
       // Script that times out when foo() is called.
-      "function foo() {while (1);}",
+      {"function foo() {while (1);}", false},
+
       // Script that times out when run and when foo is called.
-      R"(function foo() {while (1);}
+      {R"(function foo() {while (1);}
-         while(1);)",
+        while(1);)",
-  };
+       true}};
 
   // Use a sorter timeout so test runs faster.
   const base::TimeDelta kScriptTimeout = base::TimeDelta::FromMilliseconds(20);
   helper_.set_script_timeout_for_testing(kScriptTimeout);
 
-  for (const char* hanging_script : kHangingScripts) {
+  for (const HangingScript& hanging_script : kHangingScripts) {
     base::TimeTicks start_time = base::TimeTicks::Now();
     v8::Local<v8::Context> context = helper_.CreateContext();
     v8::Context::Scope context_scope(context);
 
     v8::Local<v8::UnboundScript> script;
-    ASSERT_TRUE(helper_.Compile(hanging_script, GURL("https://foo.test/"))
+    base::Optional<std::string> error_msg;
+    ASSERT_TRUE(helper_
+                    .Compile(hanging_script.script, GURL("https://foo.test/"),
+                             error_msg)
                     .ToLocal(&script));
+    EXPECT_FALSE(error_msg.has_value());
 
-    v8::MaybeLocal<v8::Value> result =
+    v8::MaybeLocal<v8::Value> result = helper_.RunScript(
-        helper_.RunScript(context, script, "foo");
+        context, script, "foo", base::span<v8::Local<v8::Value>>(), error_msg);
     EXPECT_TRUE(result.IsEmpty());
+    ASSERT_TRUE(error_msg.has_value());
+    EXPECT_EQ(hanging_script.top_level_hangs
+                  ? "https://foo.test/ top-level execution timed out."
+                  : "https://foo.test/ execution of `foo` timed out.",
+              error_msg.value());
 
     // Make sure at least `kScriptTimeout` has passed, allowing for some time
     // skew between change in base::TimeTicks::Now() and the timeout. This
@@ -93,11 +122,18 @@ TEST_F(AuctionV8HelperTest, Timeout) {
   v8::Local<v8::Context> context = helper_.CreateContext();
   v8::Context::Scope context_scope(context);
   v8::Local<v8::UnboundScript> script;
-  ASSERT_TRUE(
+  base::Optional<std::string> error_msg;
-      helper_.Compile("function foo() { return 1;}", GURL("https://foo.test/"))
+  ASSERT_TRUE(helper_
-          .ToLocal(&script));
+                  .Compile("function foo() { return 1;}",
+                           GURL("https://foo.test/"), error_msg)
+                  .ToLocal(&script));
+  EXPECT_FALSE(error_msg.has_value());
   v8::Local<v8::Value> result;
-  ASSERT_TRUE(helper_.RunScript(context, script, "foo").ToLocal(&result));
+  ASSERT_TRUE(helper_
+                  .RunScript(context, script, "foo",
+                             base::span<v8::Local<v8::Value>>(), error_msg)
+                  .ToLocal(&result));
+  EXPECT_FALSE(error_msg.has_value());
   int int_result = 0;
   ASSERT_TRUE(gin::ConvertFromV8(helper_.isolate(), result, &int_result));
   EXPECT_EQ(1, int_result);
@@ -111,11 +147,116 @@ TEST_F(AuctionV8HelperTest, NoTime) {
 
   // Make sure Date() is not accessible.
   v8::Local<v8::UnboundScript> script;
+  base::Optional<std::string> error_msg;
   ASSERT_TRUE(helper_
                   .Compile("function foo() { return Date();}",
-                           GURL("https://foo.test/"))
+                           GURL("https://foo.test/"), error_msg)
                   .ToLocal(&script));
-  EXPECT_TRUE(helper_.RunScript(context, script, "foo").IsEmpty());
+  EXPECT_FALSE(error_msg.has_value());
+  EXPECT_TRUE(helper_
+                  .RunScript(context, script, "foo",
+                             base::span<v8::Local<v8::Value>>(), error_msg)
+                  .IsEmpty());
+  ASSERT_TRUE(error_msg.has_value());
+  EXPECT_THAT(error_msg.value(), StartsWith("https://foo.test/:1"));
+  EXPECT_THAT(error_msg.value(), HasSubstr("ReferenceError"));
+  EXPECT_THAT(error_msg.value(), HasSubstr("Date"));
+}
+
+// A script that doesn't compile.
+TEST_F(AuctionV8HelperTest, CompileError) {
+  v8::Local<v8::UnboundScript> script;
+  v8::Context::Scope ctx(helper_.scratch_context());
+  base::Optional<std::string> error_msg;
+  ASSERT_FALSE(
+      helper_.Compile("function foo() { ", GURL("https://foo.test/"), error_msg)
+          .ToLocal(&script));
+  ASSERT_TRUE(error_msg.has_value());
+  EXPECT_THAT(error_msg.value(), StartsWith("https://foo.test/:1 "));
+  EXPECT_THAT(error_msg.value(), HasSubstr("SyntaxError"));
+}
+
+// Test for exception at runtime at top-level.
+TEST_F(AuctionV8HelperTest, RunErrorTopLevel) {
+  v8::Local<v8::UnboundScript> script;
+  {
+    v8::Context::Scope ctx(helper_.scratch_context());
+    base::Optional<std::string> error_msg;
+    ASSERT_TRUE(helper_
+                    .Compile("\n\nthrow new Error('I am an error');",
+                             GURL("https://foo.test/"), error_msg)
+                    .ToLocal(&script));
+    EXPECT_FALSE(error_msg.has_value());
+  }
+
+  v8::Local<v8::Context> context = helper_.CreateContext();
+  base::Optional<std::string> error_msg;
+  v8::Context::Scope ctx(context);
+  v8::Local<v8::Value> result;
+  ASSERT_FALSE(helper_
+                   .RunScript(context, script, "foo",
+                              base::span<v8::Local<v8::Value>>(), error_msg)
+                   .ToLocal(&result));
+  ASSERT_TRUE(error_msg.has_value());
+  EXPECT_EQ("https://foo.test/:3 Uncaught Error: I am an error.",
+            error_msg.value());
+}
+
+// Test for when desired function isn't found
+TEST_F(AuctionV8HelperTest, TargetFunctionNotFound) {
+  v8::Local<v8::UnboundScript> script;
+  {
+    v8::Context::Scope ctx(helper_.scratch_context());
+    base::Optional<std::string> error_msg;
+    ASSERT_TRUE(helper_
+                    .Compile("function foo() { return 1;}",
+                             GURL("https://foo.test/"), error_msg)
+                    .ToLocal(&script));
+    EXPECT_FALSE(error_msg.has_value());
+  }
+
+  v8::Local<v8::Context> context = helper_.CreateContext();
+
+  base::Optional<std::string> error_msg;
+  v8::Context::Scope ctx(context);
+  v8::Local<v8::Value> result;
+  ASSERT_FALSE(helper_
+                   .RunScript(context, script, "bar",
+                              base::span<v8::Local<v8::Value>>(), error_msg)
+                   .ToLocal(&result));
+  ASSERT_TRUE(error_msg.has_value());
+
+  // This "not a function" and not "not found" since the lookup successfully
+  // returns `undefined`.
+  EXPECT_EQ("https://foo.test/ `bar` is not a function.", error_msg.value());
+}
+
+TEST_F(AuctionV8HelperTest, TargetFunctionError) {
+  v8::Local<v8::UnboundScript> script;
+  {
+    v8::Context::Scope ctx(helper_.scratch_context());
+    base::Optional<std::string> error_msg;
+    ASSERT_TRUE(helper_
+                    .Compile("function foo() { return notfound;}",
+                             GURL("https://foo.test/"), error_msg)
+                    .ToLocal(&script));
+    EXPECT_FALSE(error_msg.has_value());
+  }
+
+  v8::Local<v8::Context> context = helper_.CreateContext();
+
+  base::Optional<std::string> error_msg;
+  v8::Context::Scope ctx(context);
+  v8::Local<v8::Value> result;
+  ASSERT_FALSE(helper_
+                   .RunScript(context, script, "foo",
+                              base::span<v8::Local<v8::Value>>(), error_msg)
+                   .ToLocal(&result));
+  ASSERT_TRUE(error_msg.has_value());
+
+  EXPECT_THAT(error_msg.value(), StartsWith("https://foo.test/:1 "));
+  EXPECT_THAT(error_msg.value(), HasSubstr("ReferenceError"));
+  EXPECT_THAT(error_msg.value(), HasSubstr("notfound"));
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/bidder_worklet.cc

@@ -14,6 +14,7 @@
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/strings/strcat.h"
 #include "base/time/time.h"
 #include "content/services/auction_worklet/auction_v8_helper.h"
 #include "content/services/auction_worklet/public/mojom/auction_worklet_service.mojom.h"
@@ -59,6 +60,17 @@ BidderWorklet::BidResult::BidResult(std::string ad, double bid, GURL render_url)
   DCHECK(this->render_url.is_valid());
 }
 
+BidderWorklet::BidResult::BidResult(base::Optional<std::string> error_msg)
+    : error_msg(std::move(error_msg)) {}
+
+BidderWorklet::BidResult::BidResult(const BidResult& other) = default;
+BidderWorklet::BidResult::BidResult(BidResult&& other) = default;
+BidderWorklet::BidResult::~BidResult() = default;
+BidderWorklet::BidResult& BidderWorklet::BidResult::operator=(
+    const BidResult&) = default;
+BidderWorklet::BidResult& BidderWorklet::BidResult::operator=(BidResult&&) =
+    default;
+
 BidderWorklet::ReportWinResult::ReportWinResult() = default;
 
 BidderWorklet::ReportWinResult::ReportWinResult(GURL report_url)
@@ -66,6 +78,20 @@ BidderWorklet::ReportWinResult::ReportWinResult(GURL report_url)
   DCHECK(this->report_url.is_valid());
 }
 
+BidderWorklet::ReportWinResult::ReportWinResult(
+    base::Optional<std::string> error_msg)
+    : error_msg(std::move(error_msg)) {}
+
+BidderWorklet::ReportWinResult::ReportWinResult(const ReportWinResult& other) =
+    default;
+BidderWorklet::ReportWinResult::ReportWinResult(ReportWinResult&& other) =
+    default;
+BidderWorklet::ReportWinResult::~ReportWinResult() = default;
+BidderWorklet::ReportWinResult& BidderWorklet::ReportWinResult::operator=(
+    const ReportWinResult&) = default;
+BidderWorklet::ReportWinResult& BidderWorklet::ReportWinResult::operator=(
+    ReportWinResult&&) = default;
+
 BidderWorklet::BidderWorklet(
     network::mojom::URLLoaderFactory* url_loader_factory,
     mojom::BiddingInterestGroupPtr bidding_interest_group,
@@ -76,7 +102,9 @@ BidderWorklet::BidderWorklet(
     base::Time auction_start_time,
     AuctionV8Helper* v8_helper,
     LoadWorkletCallback load_worklet_callback)
-    : v8_helper_(v8_helper),
+    : script_source_url_(
+          bidding_interest_group->group->bidding_url.value_or(GURL())),
+      v8_helper_(v8_helper),
       bidding_interest_group_(std::move(bidding_interest_group)),
       auction_signals_json_(auction_signals_json),
       per_buyer_signals_json_(per_buyer_signals_json),
@@ -85,11 +113,10 @@ BidderWorklet::BidderWorklet(
       browser_signal_seller_(browser_signal_seller),
       auction_start_time_(auction_start_time) {
   DCHECK(load_worklet_callback);
-  // TODO(mmenke): Remove up the value_or() - auction worklets shouldn't be
+  // TODO(mmenke): Remove up the value_or() for script_source_url_- auction
-  // created when there's no bidding URL.
+  // worklets shouldn't be created when there's no bidding URL.
   worklet_loader_ = std::make_unique<WorkletLoader>(
-      url_loader_factory,
+      url_loader_factory, script_source_url_, v8_helper,
-      bidding_interest_group_->group->bidding_url.value_or(GURL()), v8_helper,
       base::BindOnce(&BidderWorklet::OnDownloadComplete, base::Unretained(this),
                      std::move(load_worklet_callback)));
 }
@@ -202,12 +229,18 @@ BidderWorklet::BidResult BidderWorklet::GenerateBid(
   args.push_back(browser_signals);
 
   v8::Local<v8::Value> generate_bid_result;
+  base::Optional<std::string> error_msg_out;
   if (!v8_helper_
            ->RunScript(context, worklet_script_->Get(isolate), "generateBid",
-                       args)
+                       args, error_msg_out)
-           .ToLocal(&generate_bid_result) ||
+           .ToLocal(&generate_bid_result)) {
-      !generate_bid_result->IsObject()) {
+    return BidResult(std::move(error_msg_out));
-    return BidResult();
+  }
+
+  if (!generate_bid_result->IsObject()) {
+    return BidResult(
+        base::StrCat({script_source_url_.spec(),
+                      " generateBid() return value not an object."}));
   }
 
   gin::Dictionary result_dict(isolate, generate_bid_result.As<v8::Object>());
@@ -221,22 +254,29 @@ BidderWorklet::BidResult BidderWorklet::GenerateBid(
       !v8_helper_->ExtractJson(context, ad_object, &ad_json) ||
       !result_dict.Get("bid", &bid) ||
       !result_dict.Get("render", &render_url_string)) {
-    return BidResult();
+    return BidResult(
+        base::StrCat({script_source_url_.spec(),
+                      " generateBid() return value has incorrect structure."}));
   }
 
   if (bid <= 0 || std::isnan(bid) || !std::isfinite(bid))
     return BidResult();
 
   GURL render_url(render_url_string);
-  if (!render_url.is_valid() || !render_url.SchemeIs(url::kHttpsScheme))
+  if (!render_url.is_valid() || !render_url.SchemeIs(url::kHttpsScheme)) {
-    return BidResult();
+    return BidResult(base::StrCat(
+        {script_source_url_.spec(),
+         " generateBid() returned render_url isn't a valid https:// URL."}));
+  }
 
   // `render_url` must be in `ad_render_urls`.
   for (const auto& ad : *interest_group.ads) {
     if (render_url == ad->render_url)
       return BidResult(std::move(ad_json), bid, std::move(render_url));
   }
-  return BidResult();
+  return BidResult(base::StrCat({script_source_url_.spec(),
+                                 " generateBid() returned render_url isn't one "
+                                 "of the registered creative URLs."}));
 }
 
 BidderWorklet::ReportWinResult BidderWorklet::ReportWin(
@@ -285,10 +325,12 @@ BidderWorklet::ReportWinResult BidderWorklet::ReportWin(
 
   // An empty return value indicates an exception was thrown. Any other return
   // value indicates no exception.
+  base::Optional<std::string> error_msg_out;
   if (v8_helper_
-          ->RunScript(context, worklet_script_->Get(isolate), "reportWin", args)
+          ->RunScript(context, worklet_script_->Get(isolate), "reportWin", args,
+                      error_msg_out)
           .IsEmpty()) {
-    return ReportWinResult();
+    return ReportWinResult(std::move(error_msg_out));
   }
 
   if (!report_bindings.report_url().is_valid())
@@ -299,10 +341,12 @@ BidderWorklet::ReportWinResult BidderWorklet::ReportWin(
 
 void BidderWorklet::OnDownloadComplete(
     LoadWorkletCallback load_worklet_callback,
-    std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script) {
+    std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+    base::Optional<std::string> error_msg) {
   worklet_loader_.reset();
   worklet_script_ = std::move(worklet_script);
-  std::move(load_worklet_callback).Run(worklet_script_ != nullptr);
+  std::move(load_worklet_callback)
+      .Run(worklet_script_ != nullptr, std::move(error_msg));
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/bidder_worklet.h

@@ -49,12 +49,21 @@ class BidderWorklet {
     // valid.
     BidResult(std::string ad, double bid, GURL render_url);
 
+    // Constructor when there is no bid due to an error and an error message
+    // may be available.
+    explicit BidResult(base::Optional<std::string> error_msg);
+
+    BidResult(const BidResult& other);
+    BidResult(BidResult&& other);
+
+    ~BidResult();
+
+    BidResult& operator=(const BidResult&);
+    BidResult& operator=(BidResult&&);
+
     // `success` will be false on any type of failure, and other values will be
     // empty or 0. Inability to extract ad string, bid value, or valid render
     // URL are all considered errors.
-    //
-    // TODO(mmenke): Pass along some sort of error string instead, to make
-    // debugging easier.
     bool success = false;
 
     // JSON string to be passed to the scoring function.
@@ -66,6 +75,10 @@ class BidderWorklet {
 
     // Render URL, if any bid was made.
     GURL render_url;
+
+    // Error message for debugging. This isn't guaranteed to be produced for all
+    // failures, so don't check this instead of `success`.
+    base::Optional<std::string> error_msg;
   };
 
   struct ReportWinResult {
@@ -76,19 +89,33 @@ class BidderWorklet {
     // Constructor when a report was requested.
     explicit ReportWinResult(GURL report_url);
 
+    // Constructor when there is some sort of a falire for which an error
+    // message may be available.
+    explicit ReportWinResult(base::Optional<std::string> error_msg);
+
+    ReportWinResult(const ReportWinResult& other);
+    ReportWinResult(ReportWinResult&& other);
+
+    ~ReportWinResult();
+
+    ReportWinResult& operator=(const ReportWinResult&);
+    ReportWinResult& operator=(ReportWinResult&&);
+
     // `success` will be false on any type of failure. Neither lack or reporting
     // function nor lack of report URL is considered an error.
-    //
-    // TODO(mmenke): Pass along some sort of error string instead, to make
-    // debugging easier.
     bool success = false;
 
     // Report URL, if one is provided. Empty on failure, or if no report URL is
     // provided.
     GURL report_url;
+
+    // Error message for debugging.
+    base::Optional<std::string> error_msg;
   };
 
-  using LoadWorkletCallback = base::OnceCallback<void(bool success)>;
+  using LoadWorkletCallback =
+      base::OnceCallback<void(bool success,
+                              base::Optional<std::string> error_msg)>;
 
   // Starts loading the worklet script on construction. Callback will be invoked
   // asynchronously once the data has been fetched or an error has occurred.
@@ -123,8 +150,10 @@ class BidderWorklet {
  private:
   void OnDownloadComplete(
       LoadWorkletCallback load_worklet_callback,
-      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script);
+      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+      base::Optional<std::string> error_msg);
 
+  const GURL script_source_url_;
   AuctionV8Helper* const v8_helper_;
   const mojom::BiddingInterestGroupPtr bidding_interest_group_;
 

content/services/auction_worklet/bidder_worklet_unittest.cc

@@ -20,11 +20,15 @@
 #include "mojo/public/cpp/bindings/struct_ptr.h"
 #include "net/http/http_status_code.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/interest_group/interest_group_types.mojom.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
+using testing::HasSubstr;
+using testing::StartsWith;
+
 namespace auction_worklet {
 namespace {
 
@@ -135,31 +139,41 @@ class BidderWorkletTest : public testing::Test {
     EXPECT_EQ(expected_result.ad, actual_result.ad);
     EXPECT_EQ(expected_result.bid, actual_result.bid);
     EXPECT_EQ(expected_result.render_url, actual_result.render_url);
+    EXPECT_EQ(expected_result.error_msg.has_value(),
+              actual_result.error_msg.has_value());
+    EXPECT_EQ(expected_result.error_msg.value_or("Not an error"),
+              actual_result.error_msg.value_or("Not an error"));
   }
 
   // Configures `url_loader_factory_` to return a reportWin() script with the
   // specified body. Then runs the script, expecting the provided result.
   void RunReportWinWithFunctionBodyExpectingResult(
       const std::string& function_body,
-      const GURL& expected_report_url) {
+      const GURL& expected_report_url,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     RunReportWinWithJavascriptExpectingResult(
-        CreateReportWinScript(function_body), expected_report_url);
+        CreateReportWinScript(function_body), expected_report_url,
+        std::move(expected_error_msg));
   }
 
   // Configures `url_loader_factory_` to return a reportWin() script with the
   // specified Javascript. Then runs the script, expecting the provided result.
   void RunReportWinWithJavascriptExpectingResult(
       const std::string& javascript,
-      const GURL& expected_report_url) {
+      const GURL& expected_report_url,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     SCOPED_TRACE(javascript);
     AddJavascriptResponse(&url_loader_factory_, interest_group_bidding_url_,
                           javascript);
-    RunReportWinExpectingResult(expected_report_url);
+    RunReportWinExpectingResult(expected_report_url,
+                                std::move(expected_error_msg));
   }
 
   // Loads and runs a reportWin() with the provided return line, expecting the
   // supplied result.
-  void RunReportWinExpectingResult(const GURL& expected_report_url) {
+  void RunReportWinExpectingResult(
+      const GURL& expected_report_url,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     auto bidder_worket = CreateWorklet();
     ASSERT_TRUE(bidder_worket);
 
@@ -168,6 +182,10 @@ class BidderWorkletTest : public testing::Test {
         browser_signal_ad_render_fingerprint_, browser_signal_bid_);
     EXPECT_EQ(!expected_report_url.is_empty(), actual_result.success);
     EXPECT_EQ(expected_report_url, actual_result.report_url);
+    EXPECT_EQ(expected_error_msg.has_value(),
+              actual_result.error_msg.has_value());
+    EXPECT_EQ(expected_error_msg.value_or("Not an error"),
+              actual_result.error_msg.value_or("Not an error"));
   }
 
   // Create a BidderWorklet, waiting for the URLLoader to complete. Returns
@@ -231,11 +249,19 @@ class BidderWorkletTest : public testing::Test {
     return out;
   }
 
-  void CreateWorkletCallback(bool success) {
+  void CreateWorkletCallback(bool success,
+                             base::Optional<std::string> error_msg) {
     create_worklet_succeeded_ = success;
+    error_msg_ = std::move(error_msg);
+    if (success)
+      EXPECT_FALSE(error_msg_.has_value());
     load_script_run_loop_->Quit();
   }
 
+  std::string last_error_msg() const {
+    return error_msg_.value_or("Not an error");
+  }
+
   base::test::TaskEnvironment task_environment_;
 
   // Values used to construct the BiddingInterestGroup passed to the
@@ -277,6 +303,7 @@ class BidderWorkletTest : public testing::Test {
   // synchronously.
   std::unique_ptr<base::RunLoop> load_script_run_loop_;
   bool create_worklet_succeeded_ = false;
+  base::Optional<std::string> error_msg_;
 
   network::TestURLLoaderFactory url_loader_factory_;
   AuctionV8Helper v8_helper_;
@@ -287,12 +314,17 @@ TEST_F(BidderWorkletTest, NetworkError) {
                                   CreateBasicGenerateBidScript(),
                                   net::HTTP_NOT_FOUND);
   EXPECT_FALSE(CreateWorklet());
+  EXPECT_EQ("Failed to load https://url.test/ HTTP status = 404 Not Found.",
+            last_error_msg());
 }
 
 TEST_F(BidderWorkletTest, CompileError) {
   AddJavascriptResponse(&url_loader_factory_, interest_group_bidding_url_,
                         "Invalid Javascript");
   EXPECT_FALSE(CreateWorklet());
+
+  EXPECT_THAT(last_error_msg(), StartsWith("https://url.test/:1 "));
+  EXPECT_THAT(last_error_msg(), HasSubstr("SyntaxError"));
 }
 
 // Test parsing of return values.
@@ -335,10 +367,12 @@ TEST_F(BidderWorkletTest, GenerateBidResult) {
   // Other values JSON can't represent result in failing instead of null.
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: globalThis.not_defined, bid:1, render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: function() {return 1;}, bid:1, render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
 
   // Make sure recursive structures aren't allowed in ad field.
   RunGenerateBidWithJavascriptExpectingResult(
@@ -349,7 +383,8 @@ TEST_F(BidderWorkletTest, GenerateBidResult) {
           return {ad: a, bid:1, render:"https://response.test/"};
         }
       )",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
 
   // --------
   // Vary bid
@@ -392,10 +427,12 @@ TEST_F(BidderWorkletTest, GenerateBidResult) {
   // Non-numeric bid.
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:"1", render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:[1], render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
 
   // ---------
   // Vary URL.
@@ -408,43 +445,61 @@ TEST_F(BidderWorkletTest, GenerateBidResult) {
   // Disallowed schemes.
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:1, render:"http://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:1, render:"chrome-extension://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:1, render:"about:blank"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
   RunGenerateBidWithReturnValueExpectingResult(
-      R"({ad: ["ad"], bid:1, render:"data:,foo"})", BidderWorklet::BidResult());
+      R"({ad: ["ad"], bid:1, render:"data:,foo"})",
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
 
   // Invalid URLs.
   RunGenerateBidWithReturnValueExpectingResult(
-      R"({ad: ["ad"], bid:1, render:"test"})", BidderWorklet::BidResult());
+      R"({ad: ["ad"], bid:1, render:"test"})",
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
   RunGenerateBidWithReturnValueExpectingResult(
-      R"({ad: ["ad"], bid:1, render:"http://"})", BidderWorklet::BidResult());
+      R"({ad: ["ad"], bid:1, render:"http://"})",
+      BidderWorklet::BidResult("https://url.test/ generateBid() returned "
+                               "render_url isn't a valid https:// URL."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], bid:1, render:["http://response.test/"]})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
   RunGenerateBidWithReturnValueExpectingResult(
-      R"({ad: ["ad"], bid:1, render:9})", BidderWorklet::BidResult());
+      R"({ad: ["ad"], bid:1, render:9})",
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
 
   // ------------
   // Other cases.
   // ------------
 
   // No return value.
-  RunGenerateBidWithReturnValueExpectingResult("", BidderWorklet::BidResult());
+  RunGenerateBidWithReturnValueExpectingResult(
+      "", BidderWorklet::BidResult(
+              "https://url.test/ generateBid() return value not an object."));
 
   // Missing value.
   RunGenerateBidWithReturnValueExpectingResult(
       R"({bid:"a", render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: ["ad"], render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
-  RunGenerateBidWithReturnValueExpectingResult(R"({ad: ["ad"], bid:"a"})",
+                               "has incorrect structure."));
-                                               BidderWorklet::BidResult());
+  RunGenerateBidWithReturnValueExpectingResult(
+      R"({ad: ["ad"], bid:"a"})",
+      BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+                               "has incorrect structure."));
 
   // Valid JS, but missing function.
   RunGenerateBidWithJavascriptExpectingResult(
@@ -453,22 +508,28 @@ TEST_F(BidderWorkletTest, GenerateBidResult) {
           return {ad: ["ad"], bid:1, render:"https://response.test/"};
         }
       )",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult(
-  RunGenerateBidWithJavascriptExpectingResult("", BidderWorklet::BidResult());
+          "https://url.test/ `generateBid` is not a function."));
-  RunGenerateBidWithJavascriptExpectingResult("5", BidderWorklet::BidResult());
+  RunGenerateBidWithJavascriptExpectingResult(
-  RunGenerateBidWithJavascriptExpectingResult("shrimp",
+      "", BidderWorklet::BidResult(
-                                              BidderWorklet::BidResult());
+              "https://url.test/ `generateBid` is not a function."));
+  RunGenerateBidWithJavascriptExpectingResult(
+      "5", BidderWorklet::BidResult(
+               "https://url.test/ `generateBid` is not a function."));
 
   // Throw exception.
-  RunGenerateBidWithReturnValueExpectingResult("shrimp",
+  RunGenerateBidWithJavascriptExpectingResult(
-                                               BidderWorklet::BidResult());
+      "shrimp",
+      BidderWorklet::BidResult("https://url.test/:1 Uncaught ReferenceError: "
+                               "shrimp is not defined."));
 }
 
 // Make sure Date() is not available when running generateBid().
 TEST_F(BidderWorkletTest, GenerateBidDateNotAvailable) {
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: Date().toString(), bid:1, render:"https://response.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult(
+          "https://url.test/:4 Uncaught ReferenceError: Date is not defined."));
 }
 
 // Checks that most input parameters are correctly passed in, and each is parsed
@@ -610,7 +671,9 @@ TEST_F(BidderWorkletTest, GenerateBidBasicInputParameters) {
   // A bid URL that's not in the InterestGroup's ads list should fail.
   RunGenerateBidWithReturnValueExpectingResult(
       R"({ad: 0, bid:1, render:"https://response2.test/"})",
-      BidderWorklet::BidResult());
+      BidderWorklet::BidResult(
+          "https://url.test/ generateBid() returned render_url isn't one of "
+          "the registered creative URLs."));
 
   // Adding an ad with a corresponding `renderUrl` should result in success.
   // Also check the `interestGroup.ads` field passed to Javascript.
@@ -784,10 +847,12 @@ TEST_F(BidderWorkletTest, GenerateBidTrustedBiddingSignals) {
   trusted_bidding_signals_ = std::make_unique<TrustedBiddingSignals>(
       &url_loader_factory_, std::vector<std::string>({"key1", "key2"}),
       "hostname", kBaseSignalsUrl, &v8_helper_,
-      base::BindLambdaForTesting([&](bool success) {
+      base::BindLambdaForTesting(
-        signals_loaded_successfully = success;
+          [&](bool success, base::Optional<std::string> signals_error_msg) {
-        run_loop.Quit();
+            signals_loaded_successfully = success;
-      }));
+            EXPECT_FALSE(signals_error_msg.has_value());
+            run_loop.Quit();
+          }));
   run_loop.Run();
   ASSERT_TRUE(signals_loaded_successfully);
 
@@ -827,21 +892,31 @@ TEST_F(BidderWorkletTest, ReportWin) {
       R"(sendReportTo("https://foo.test/bar"))", GURL("https://foo.test/bar"));
 
   RunReportWinWithFunctionBodyExpectingResult(
-      R"(sendReportTo("http://http.not.allowed.test"))", GURL());
+      R"(sendReportTo("http://http.not.allowed.test"))", GURL(),
+      "https://url.test/:4 Uncaught TypeError: sendReportTo must be passed a "
+      "valid HTTPS url.");
   RunReportWinWithFunctionBodyExpectingResult(
-      R"(sendReportTo("file:///file.not.allowed.test"))", GURL());
+      R"(sendReportTo("file:///file.not.allowed.test"))", GURL(),
+      "https://url.test/:4 Uncaught TypeError: sendReportTo must be passed a "
+      "valid HTTPS url.");
 
-  RunReportWinWithFunctionBodyExpectingResult(R"(sendReportTo(""))", GURL());
+  RunReportWinWithFunctionBodyExpectingResult(
+      R"(sendReportTo(""))", GURL(),
+      "https://url.test/:4 Uncaught TypeError: sendReportTo must be passed a "
+      "valid HTTPS url.");
 
   RunReportWinWithFunctionBodyExpectingResult(
       R"(sendReportTo("https://foo.test");sendReportTo("https://foo.test"))",
-      GURL());
+      GURL(),
+      "https://url.test/:4 Uncaught TypeError: sendReportTo may be called at "
+      "most once.");
 }
 
 // Make sure Date() is not available when running reportWin().
 TEST_F(BidderWorkletTest, ReportWinDateNotAvailable) {
   RunReportWinWithFunctionBodyExpectingResult(
-      R"(sendReportTo("https://foo.test/" + Date().toString()))", GURL());
+      R"(sendReportTo("https://foo.test/" + Date().toString()))", GURL(),
+      "https://url.test/:4 Uncaught ReferenceError: Date is not defined.");
 }
 
 TEST_F(BidderWorkletTest, ReportWinParameters) {
@@ -852,31 +927,49 @@ TEST_F(BidderWorkletTest, ReportWinParameters) {
     bool is_json;
     // Pointer to location at which the string can be modified.
     std::string* value_ptr;
+    // Whether to expect an error. This can be empty when call fails in case
+    // it's due to something like passing non-JSON to JSON parameter which user
+    // code should be unable to trigger, and for which we thus do not produce
+    // an error message.
+    base::Optional<std::string> expect_error_msg;
+    base::Optional<std::string> expect_error_msg_array;
   } kStringTestCases[] = {
       {
           "auctionSignals",
           true /* is_json */,
           &auction_signals_,
+          base::nullopt,
+          base::nullopt,
       },
       {
           "perBuyerSignals",
           true /* is_json */,
           &per_buyer_signals_,
+          base::nullopt,
+          base::nullopt,
       },
       {
           "sellerSignals",
           true /* is_json */,
           &seller_signals_,
+          base::nullopt,
+          base::nullopt,
       },
       {
           "browserSignals.interestGroupName",
           false /* is_json */,
           &interest_group_name_,
+          base::nullopt,
+          "https://url.test/:4 Uncaught TypeError: sendReportTo must be passed "
+          "a valid HTTPS url.",
       },
       {
           "browserSignals.adRenderFingerprint",
           false /* is_json */,
           &browser_signal_ad_render_fingerprint_,
+          base::nullopt,
+          "https://url.test/:4 Uncaught TypeError: sendReportTo must be passed "
+          "a valid HTTPS url.",
       },
   };
 
@@ -886,12 +979,14 @@ TEST_F(BidderWorkletTest, ReportWinParameters) {
     *test_case.value_ptr = "https://foo.test/";
     RunReportWinWithFunctionBodyExpectingResult(
         base::StringPrintf("sendReportTo(%s)", test_case.name),
-        test_case.is_json ? GURL() : GURL("https://foo.test/"));
+        test_case.is_json ? GURL() : GURL("https://foo.test/"),
+        test_case.expect_error_msg);
 
     *test_case.value_ptr = R"(["https://foo.test/"])";
     RunReportWinWithFunctionBodyExpectingResult(
         base::StringPrintf("sendReportTo(%s[0])", test_case.name),
-        test_case.is_json ? GURL("https://foo.test/") : GURL());
+        test_case.is_json ? GURL("https://foo.test/") : GURL(),
+        test_case.expect_error_msg_array);
 
     SetDefaultParameters();
   }

content/services/auction_worklet/public/mojom/auction_worklet_service.mojom

@@ -106,6 +106,9 @@ interface AuctionWorkletService {
   // `seller_report` indicates if the seller wishes to make a report,
   //  and the URL that the seller wanted fetched for that.
   //
+  // `errors` are various error messages to be used for debugging. These are too
+  // sensitive for the renderers to see.
+  //
   // TODO(mmenke):  May be good to make some way to share worklets between
   // multiple auctions on the same page, but not auctions across pages.
   // Could create separate top level AuctionWorkletService objects (using the
@@ -119,5 +122,6 @@ interface AuctionWorkletService {
                 url.mojom.Origin winning_interest_group_owner,
                 string winning_interest_group_name,
                 WinningBidderReport bidder_report,
-                SellerReport seller_report);
+                SellerReport seller_report,
+                array<string> errors);
 };

content/services/auction_worklet/report_bindings.cc

@@ -45,7 +45,7 @@ void ReportBindings::SendReportTo(
     bindings->report_url_ = GURL();
     args.GetIsolate()->ThrowException(
         v8::Exception::TypeError(v8_helper->CreateStringFromLiteral(
-            "SendReportTo requires 1 string parameter.")));
+            "sendReportTo requires 1 string parameter")));
     return;
   }
 
@@ -54,7 +54,8 @@ void ReportBindings::SendReportTo(
     bindings->report_url_ = GURL();
     args.GetIsolate()->ThrowException(
         v8::Exception::TypeError(v8_helper->CreateStringFromLiteral(
-            "SendReportTo may be called at most once.")));
+            "sendReportTo may be called at most once")));
+    return;
   }
 
   GURL url(url_string);
@@ -62,7 +63,7 @@ void ReportBindings::SendReportTo(
     bindings->exception_thrown_ = true;
     args.GetIsolate()->ThrowException(
         v8::Exception::TypeError(v8_helper->CreateStringFromLiteral(
-            "SendReportTo must be passed a valid HTTPS url.")));
+            "sendReportTo must be passed a valid HTTPS url")));
     return;
   }
 

content/services/auction_worklet/seller_worklet.cc

@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/strings/strcat.h"
 #include "base/time/time.h"
 #include "content/services/auction_worklet/auction_v8_helper.h"
 #include "content/services/auction_worklet/public/mojom/auction_worklet_service.mojom.h"
@@ -114,6 +115,17 @@ SellerWorklet::ScoreResult::ScoreResult(double score)
   DCHECK_GT(score, 0);
 }
 
+SellerWorklet::ScoreResult::ScoreResult(base::Optional<std::string> error_msg)
+    : error_msg(std::move(error_msg)) {}
+
+SellerWorklet::ScoreResult::ScoreResult(const ScoreResult& other) = default;
+SellerWorklet::ScoreResult::ScoreResult(ScoreResult&& other) = default;
+SellerWorklet::ScoreResult::~ScoreResult() = default;
+SellerWorklet::ScoreResult& SellerWorklet::ScoreResult::operator=(
+    const ScoreResult&) = default;
+SellerWorklet::ScoreResult& SellerWorklet::ScoreResult::operator=(
+    ScoreResult&&) = default;
+
 SellerWorklet::Report::Report() = default;
 
 SellerWorklet::Report::Report(std::string signals_for_winner, GURL report_url)
@@ -121,12 +133,22 @@ SellerWorklet::Report::Report(std::string signals_for_winner, GURL report_url)
       signals_for_winner(std::move(signals_for_winner)),
       report_url(std::move(report_url)) {}
 
+SellerWorklet::Report::Report(base::Optional<std::string> error_msg)
+    : error_msg(std::move(error_msg)) {}
+
+SellerWorklet::Report::Report(const Report& other) = default;
+SellerWorklet::Report::Report(Report&& other) = default;
+SellerWorklet::Report::~Report() = default;
+SellerWorklet::Report& SellerWorklet::Report::operator=(const Report&) =
+    default;
+SellerWorklet::Report& SellerWorklet::Report::operator=(Report&&) = default;
+
 SellerWorklet::SellerWorklet(
     network::mojom::URLLoaderFactory* url_loader_factory,
     const GURL& script_source_url,
     AuctionV8Helper* v8_helper,
     LoadWorkletCallback load_worklet_callback)
-    : v8_helper_(v8_helper) {
+    : script_source_url_(script_source_url), v8_helper_(v8_helper) {
   DCHECK(load_worklet_callback);
   worklet_loader_ = std::make_unique<WorkletLoader>(
       url_loader_factory, script_source_url, v8_helper,
@@ -181,14 +203,22 @@ SellerWorklet::ScoreResult SellerWorklet::ScoreAd(
 
   v8::Local<v8::Value> score_ad_result;
   double score;
+  base::Optional<std::string> error_msg_out;
   if (!v8_helper_
-           ->RunScript(context, worklet_script_->Get(isolate), "scoreAd", args)
+           ->RunScript(context, worklet_script_->Get(isolate), "scoreAd", args,
-           .ToLocal(&score_ad_result) ||
+                       error_msg_out)
-      !gin::ConvertFromV8(isolate, score_ad_result, &score)) {
+           .ToLocal(&score_ad_result)) {
-    return ScoreResult();
+    return ScoreResult(std::move(error_msg_out));
   }
 
-  if (score <= 0 || std::isnan(score) || !std::isfinite(score))
+  if (!gin::ConvertFromV8(isolate, score_ad_result, &score) ||
+      std::isnan(score) || !std::isfinite(score)) {
+    return ScoreResult(
+        base::StrCat({script_source_url_.spec(),
+                      " scoreAd() did not return a valid number."}));
+  }
+
+  if (score <= 0)
     return ScoreResult();
 
   return ScoreResult(score);
@@ -236,11 +266,12 @@ SellerWorklet::Report SellerWorklet::ReportResult(
   args.push_back(browser_signals);
 
   v8::Local<v8::Value> signals_for_winner_value;
+  base::Optional<std::string> error_msg_out;
   if (!v8_helper_
            ->RunScript(context, worklet_script_->Get(isolate), "reportResult",
-                       args)
+                       args, error_msg_out)
            .ToLocal(&signals_for_winner_value)) {
-    return Report();
+    return Report(std::move(error_msg_out));
   }
 
   // Consider lack of error but no return value type, or a return value that
@@ -256,10 +287,12 @@ SellerWorklet::Report SellerWorklet::ReportResult(
 
 void SellerWorklet::OnDownloadComplete(
     LoadWorkletCallback load_worklet_callback,
-    std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script) {
+    std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+    base::Optional<std::string> error_msg) {
   worklet_loader_.reset();
   worklet_script_ = std::move(worklet_script);
-  std::move(load_worklet_callback).Run(worklet_script_ != nullptr);
+  std::move(load_worklet_callback)
+      .Run(worklet_script_ != nullptr, std::move(error_msg));
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/seller_worklet.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "content/services/auction_worklet/public/mojom/auction_worklet_service.mojom-forward.h"
 #include "services/network/public/mojom/url_loader_factory.mojom-forward.h"
@@ -27,8 +28,7 @@ class WorkletLoader;
 // seller worklet's Javascript.
 class SellerWorklet {
  public:
-  // The result of invoking scoreAd(). This could just be a double, but planning
+  // The result of invoking scoreAd().
-  // to add error information down the line.
   struct ScoreResult {
     // Creates a ScoreResult for a failure or rejected bid.
     ScoreResult();
@@ -36,15 +36,28 @@ class SellerWorklet {
     // Creates a ScoreResult for a successfully scored ad. `score` will be > 0.
     explicit ScoreResult(double score);
 
+    // Creates a ScoreResult representing a fatal error, potentially with a
+    // helpful diagnostic message in `error_msg`.
+    explicit ScoreResult(base::Optional<std::string> error_msg);
+
+    ScoreResult(const ScoreResult& other);
+    ScoreResult(ScoreResult&& other);
+
+    ~ScoreResult();
+
+    ScoreResult& operator=(const ScoreResult&);
+    ScoreResult& operator=(ScoreResult&&);
+
     // `success` will be false on any type of failure, and score will be 0.
-    //
-    // TODO(mmenke): Pass along some sort of error string instead, to make
-    // debugging easier.
     bool success = false;
 
     // Score. 0 if no bid is offered (even if underlying script returned a
     // negative value).
     double score = 0;
+
+    // Error message for debugging. This is not guaranteed to be present on
+    // failure.
+    base::Optional<std::string> error_msg;
   };
 
   // The result of invoking reportResult().
@@ -55,11 +68,20 @@ class SellerWorklet {
     // Creates a Report for a successful call.
     Report(std::string signals_for_winner, GURL report_url);
 
+    // Creates a ScoreResult representing a fatal error, potentially with a
+    // helpful diagnostic message in `error_msg`.
+    explicit Report(base::Optional<std::string> error_msg);
+
+    Report(const Report& other);
+    Report(Report&& other);
+
+    ~Report();
+
+    Report& operator=(const Report&);
+    Report& operator=(Report&&);
+
     // `success` will be false on any type of failure, including lack of a
     // method.
-    //
-    // TODO(mmenke): Pass along some sort of error string instead, to make
-    // debugging easier.
     bool success = false;
 
     // JSON data as a string. Sent to the winner's ReportWin function. JSON
@@ -69,9 +91,15 @@ class SellerWorklet {
     // Report URL, if one is provided. Empty on failure, or if no report URL is
     // provided.
     GURL report_url;
+
+    // Error message for debugging. This isn't guaranteed to have a value for
+    // all failures.
+    base::Optional<std::string> error_msg;
   };
 
-  using LoadWorkletCallback = base::OnceCallback<void(bool success)>;
+  using LoadWorkletCallback =
+      base::OnceCallback<void(bool success,
+                              base::Optional<std::string> error_msg)>;
 
   // Starts loading the worklet script on construction. Callback will be invoked
   // asynchronously once the data has been fetched or an error has occurred.
@@ -109,8 +137,10 @@ class SellerWorklet {
  private:
   void OnDownloadComplete(
       LoadWorkletCallback load_worklet_callback,
-      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script);
+      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+      base::Optional<std::string> error_msg);
 
+  const GURL script_source_url_;
   AuctionV8Helper* const v8_helper_;
   std::unique_ptr<WorkletLoader> worklet_loader_;
 

content/services/auction_worklet/seller_worklet_unittest.cc

@@ -18,9 +18,13 @@
 #include "content/services/auction_worklet/worklet_test_util.h"
 #include "net/http/http_status_code.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+using testing::HasSubstr;
+using testing::StartsWith;
+
 namespace auction_worklet {
 namespace {
 
@@ -85,23 +89,29 @@ class SellerWorkletTest : public testing::Test {
   // return line, expecting the provided result.
   void RunScoreAdWithReturnValueExpectingResult(
       const std::string& raw_return_value,
-      double expected_score) {
+      double expected_score,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     RunScoreAdWithJavascriptExpectingResult(
-        CreateScoreAdScript(raw_return_value), expected_score);
+        CreateScoreAdScript(raw_return_value), expected_score,
+        std::move(expected_error_msg));
   }
 
   // Configures `url_loader_factory_` to return the provided script, and then
   // runs its generate_bid() function. Then runs the script, expecting the
   // provided result.
-  void RunScoreAdWithJavascriptExpectingResult(const std::string& javascript,
+  void RunScoreAdWithJavascriptExpectingResult(
-                                               double expected_score) {
+      const std::string& javascript,
+      double expected_score,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     SCOPED_TRACE(javascript);
     AddJavascriptResponse(&url_loader_factory_, url_, javascript);
-    RunScoreAdExpectingResult(expected_score);
+    RunScoreAdExpectingResult(expected_score, std::move(expected_error_msg));
   }
 
   // Loads and runs a scode_ad() script, expecting the supplied result.
-  void RunScoreAdExpectingResult(double expected_score) {
+  void RunScoreAdExpectingResult(
+      double expected_score,
+      base::Optional<std::string> expected_error_msg = base::nullopt) {
     auto seller_worket = CreateWorklet();
     ASSERT_TRUE(seller_worket);
 
@@ -113,6 +123,10 @@ class SellerWorkletTest : public testing::Test {
                                browser_signal_bidding_duration_);
     EXPECT_EQ(expected_score > 0, actual_result.success);
     EXPECT_EQ(expected_score, actual_result.score);
+    EXPECT_EQ(expected_error_msg.has_value(),
+              actual_result.error_msg.has_value());
+    EXPECT_EQ(expected_error_msg.value_or("Not an error"),
+              actual_result.error_msg.value_or("Not an error"));
   }
 
   // Configures `url_loader_factory_` to return a report_result() script created
@@ -152,6 +166,10 @@ class SellerWorkletTest : public testing::Test {
     EXPECT_EQ(expected_report.signals_for_winner,
               actual_result.signals_for_winner);
     EXPECT_EQ(expected_report.report_url, actual_result.report_url);
+    EXPECT_EQ(expected_report.error_msg.has_value(),
+              actual_result.error_msg.has_value());
+    EXPECT_EQ(expected_report.error_msg.value_or("Not an error"),
+              actual_result.error_msg.value_or("Not an error"));
   }
 
   // Create a SellerWorklet, waiting for the URLLoader to complete. Returns
@@ -160,7 +178,7 @@ class SellerWorkletTest : public testing::Test {
     CHECK(!load_script_run_loop_);
 
     create_worklet_succeeded_ = false;
-    auto bidder_worket = std::make_unique<SellerWorklet>(
+    auto seller_worket = std::make_unique<SellerWorklet>(
         &url_loader_factory_, url_, &v8_helper_,
         base::BindOnce(&SellerWorkletTest::CreateWorkletCallback,
                        base::Unretained(this)));
@@ -169,15 +187,21 @@ class SellerWorkletTest : public testing::Test {
     load_script_run_loop_.reset();
     if (!create_worklet_succeeded_)
       return nullptr;
-    return bidder_worket;
+    return seller_worket;
   }
 
  protected:
-  void CreateWorkletCallback(bool success) {
+  void CreateWorkletCallback(bool success,
+                             base::Optional<std::string> error_msg) {
     create_worklet_succeeded_ = success;
+    error_msg_ = std::move(error_msg);
+    if (success)
+      EXPECT_FALSE(error_msg_.has_value());
     load_script_run_loop_->Quit();
   }
 
+  std::string last_error_msg() { return error_msg_.value_or("Not an error"); }
+
   base::test::TaskEnvironment task_environment_;
 
   const GURL url_ = GURL("https://url.test/");
@@ -201,6 +225,7 @@ class SellerWorkletTest : public testing::Test {
   // synchronously.
   std::unique_ptr<base::RunLoop> load_script_run_loop_;
   bool create_worklet_succeeded_ = false;
+  base::Optional<std::string> error_msg_;
 
   network::TestURLLoaderFactory url_loader_factory_;
   AuctionV8Helper v8_helper_;
@@ -210,11 +235,15 @@ TEST_F(SellerWorkletTest, NetworkError) {
   url_loader_factory_.AddResponse(url_.spec(), CreateBasicSellAdScript(),
                                   net::HTTP_NOT_FOUND);
   EXPECT_FALSE(CreateWorklet());
+  EXPECT_EQ("Failed to load https://url.test/ HTTP status = 404 Not Found.",
+            last_error_msg());
 }
 
 TEST_F(SellerWorkletTest, CompileError) {
   AddJavascriptResponse(&url_loader_factory_, url_, "Invalid Javascript");
   EXPECT_FALSE(CreateWorklet());
+  EXPECT_THAT(last_error_msg(), StartsWith("https://url.test/:1 "));
+  EXPECT_THAT(last_error_msg(), HasSubstr("SyntaxError"));
 }
 
 // Test parsing of return values.
@@ -229,21 +258,31 @@ TEST_F(SellerWorkletTest, ScoreAd) {
   RunScoreAdWithReturnValueExpectingResult("-10", 0);
 
   // No return value.
-  RunScoreAdWithReturnValueExpectingResult("", 0);
+  RunScoreAdWithReturnValueExpectingResult(
+      "", 0, "https://url.test/ scoreAd() did not return a valid number.");
 
   // Wrong return type / invalid values.
-  RunScoreAdWithReturnValueExpectingResult("[15]", 0);
+  RunScoreAdWithReturnValueExpectingResult(
-  RunScoreAdWithReturnValueExpectingResult("1/0", 0);
+      "[15]", 0, "https://url.test/ scoreAd() did not return a valid number.");
-  RunScoreAdWithReturnValueExpectingResult("0/0", 0);
+  RunScoreAdWithReturnValueExpectingResult(
-  RunScoreAdWithReturnValueExpectingResult("-1/0", 0);
+      "1/0", 0, "https://url.test/ scoreAd() did not return a valid number.");
-  RunScoreAdWithReturnValueExpectingResult("true", 0);
+  RunScoreAdWithReturnValueExpectingResult(
+      "0/0", 0, "https://url.test/ scoreAd() did not return a valid number.");
+  RunScoreAdWithReturnValueExpectingResult(
+      "-1/0", 0, "https://url.test/ scoreAd() did not return a valid number.");
+  RunScoreAdWithReturnValueExpectingResult(
+      "true", 0, "https://url.test/ scoreAd() did not return a valid number.");
 
   // Throw exception.
-  RunScoreAdWithReturnValueExpectingResult("shrimp", 0);
+  RunScoreAdWithReturnValueExpectingResult(
+      "shrimp", 0,
+      "https://url.test/:4 Uncaught ReferenceError: shrimp is not defined.");
 }
 
 TEST_F(SellerWorkletTest, ScoreAdDateNotAvailable) {
-  RunScoreAdWithReturnValueExpectingResult("Date.parse(Date().toString())", 0);
+  RunScoreAdWithReturnValueExpectingResult(
+      "Date.parse(Date().toString())", 0,
+      "https://url.test/:4 Uncaught ReferenceError: Date is not defined.");
 }
 
 // Checks that input parameters are correctly passed in.
@@ -363,7 +402,9 @@ TEST_F(SellerWorkletTest, ReportResult) {
 
   // Throw exception.
   RunReportResultCreatedScriptExpectingResult(
-      "shrimp", std::string() /* extra_code */, SellerWorklet::Report());
+      "shrimp", std::string() /* extra_code */,
+      SellerWorklet::Report("https://url.test/:4 Uncaught ReferenceError: "
+                            "shrimp is not defined."));
 }
 
 // Tests reporting URLs.
@@ -377,29 +418,49 @@ TEST_F(SellerWorkletTest, ReportResultSendReportTo) {
 
   // Disallowed schemes.
   RunReportResultCreatedScriptExpectingResult(
-      "1", R"(sendReportTo("http://foo.test/"))", SellerWorklet::Report());
+      "1", R"(sendReportTo("http://foo.test/"))",
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+                            "sendReportTo must be passed a valid HTTPS url."));
   RunReportResultCreatedScriptExpectingResult(
-      "1", R"(sendReportTo("file:///foo/"))", SellerWorklet::Report());
+      "1", R"(sendReportTo("file:///foo/"))",
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+                            "sendReportTo must be passed a valid HTTPS url."));
 
   // Multiple calls.
   RunReportResultCreatedScriptExpectingResult(
       "1",
       R"(sendReportTo("https://foo.test/"); sendReportTo("https://foo.test/"))",
-      SellerWorklet::Report());
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+                            "sendReportTo may be called at most once."));
+
+  // No message if caught, but still no URL.
+  RunReportResultCreatedScriptExpectingResult(
+      "1",
+      R"(try {
+        sendReportTo("https://foo.test/");
+        sendReportTo("https://foo.test/")} catch(e) {})",
+      SellerWorklet::Report("1", GURL()));
 
   // Not a URL.
-  RunReportResultCreatedScriptExpectingResult("1", R"(sendReportTo("France"))",
+  RunReportResultCreatedScriptExpectingResult(
-                                              SellerWorklet::Report());
+      "1", R"(sendReportTo("France"))",
-  RunReportResultCreatedScriptExpectingResult("1", R"(sendReportTo(null))",
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
-                                              SellerWorklet::Report());
+                            "sendReportTo must be passed a valid HTTPS url."));
-  RunReportResultCreatedScriptExpectingResult("1", R"(sendReportTo([5]))",
+  RunReportResultCreatedScriptExpectingResult(
-                                              SellerWorklet::Report());
+      "1", R"(sendReportTo(null))",
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+                            "sendReportTo requires 1 string parameter."));
+  RunReportResultCreatedScriptExpectingResult(
+      "1", R"(sendReportTo([5]))",
+      SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+                            "sendReportTo requires 1 string parameter."));
 }
 
 TEST_F(SellerWorkletTest, ReportResultDateNotAvailable) {
   RunReportResultCreatedScriptExpectingResult(
       "1", R"(sendReportTo("https://foo.test/" + Date().toString()))",
-      SellerWorklet::Report());
+      SellerWorklet::Report(
+          "https://url.test/:3 Uncaught ReferenceError: Date is not defined."));
 }
 
 TEST_F(SellerWorkletTest, ReportResultParameters) {

content/services/auction_worklet/trusted_bidding_signals.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
+#include "base/strings/strcat.h"
 #include "content/services/auction_worklet/auction_downloader.h"
 #include "content/services/auction_worklet/auction_v8_helper.h"
 #include "gin/converter.h"
@@ -28,7 +29,8 @@ TrustedBiddingSignals::TrustedBiddingSignals(
     const GURL& trusted_bidding_signals_url,
     AuctionV8Helper* v8_helper,
     LoadSignalsCallback load_signals_callback)
-    : v8_helper_(v8_helper),
+    : trusted_bidding_signals_url_(trusted_bidding_signals_url),
+      v8_helper_(v8_helper),
       load_signals_callback_(std::move(load_signals_callback)) {
   DCHECK(!trusted_bidding_signals_keys.empty());
   DCHECK(load_signals_callback_);
@@ -81,14 +83,17 @@ v8::Local<v8::Object> TrustedBiddingSignals::GetSignals(
 
 void TrustedBiddingSignals::OnDownloadComplete(
     std::vector<std::string> trusted_bidding_signals_keys,
-    std::unique_ptr<std::string> body) {
+    std::unique_ptr<std::string> body,
+    base::Optional<std::string> error_msg) {
   auction_downloader_.reset();
 
   if (!body) {
-    std::move(load_signals_callback_).Run(false);
+    std::move(load_signals_callback_).Run(false, std::move(error_msg));
     return;
   }
 
+  DCHECK(!error_msg.has_value());
+
   AuctionV8Helper::FullIsolateScope isolate_scope(v8_helper_);
   v8::Context::Scope context_scope(v8_helper_->scratch_context());
 
@@ -96,7 +101,9 @@ void TrustedBiddingSignals::OnDownloadComplete(
   if (!v8_helper_->CreateValueFromJson(v8_helper_->scratch_context(), *body)
            .ToLocal(&v8_data) ||
       !v8_data->IsObject()) {
-    std::move(load_signals_callback_).Run(false);
+    std::string error = base::StrCat({trusted_bidding_signals_url_.spec(),
+                                      " Unable to parse as a JSON object."});
+    std::move(load_signals_callback_).Run(false, std::move(error));
     return;
   }
 
@@ -108,7 +115,7 @@ void TrustedBiddingSignals::OnDownloadComplete(
     v8::Local<v8::Value> v8_string_value;
     std::string value;
     if (!v8_helper_->CreateUtf8String(key).ToLocal(&v8_key)) {
-      std::move(load_signals_callback_).Run(false);
+      std::move(load_signals_callback_).Run(false, base::nullopt);
       return;
     }
     // Only the `has_result` check should be able to fail.
@@ -124,7 +131,7 @@ void TrustedBiddingSignals::OnDownloadComplete(
     }
     json_data_[key] = std::move(value);
   }
-  std::move(load_signals_callback_).Run(true);
+  std::move(load_signals_callback_).Run(true, base::nullopt);
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/trusted_bidding_signals.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/optional.h"
 #include "services/network/public/mojom/url_loader_factory.mojom-forward.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
@@ -33,7 +34,9 @@ class AuctionV8Helper;
 // clone operation as a copy).
 class TrustedBiddingSignals {
  public:
-  using LoadSignalsCallback = base::OnceCallback<void(bool success)>;
+  using LoadSignalsCallback =
+      base::OnceCallback<void(bool success,
+                              base::Optional<std::string> error_msg)>;
 
   // Starts loading the JSON data on construction. `trusted_bidding_signals_url`
   // must be the base URL (no query params added). Callback will be invoked
@@ -62,8 +65,10 @@ class TrustedBiddingSignals {
 
  private:
   void OnDownloadComplete(std::vector<std::string> trusted_bidding_signals_keys,
-                          std::unique_ptr<std::string> body);
+                          std::unique_ptr<std::string> body,
+                          base::Optional<std::string> error_msg);
 
+  const GURL trusted_bidding_signals_url_;  // original, for error messages.
   AuctionV8Helper* const v8_helper_;
 
   LoadSignalsCallback load_signals_callback_;

content/services/auction_worklet/trusted_bidding_signals_unittest.cc

@@ -98,8 +98,11 @@ class TrustedBiddingSignalsTest : public testing::Test {
   }
 
  protected:
-  void LoadSignalsCallback(bool success) {
+  void LoadSignalsCallback(bool success,
+                           base::Optional<std::string> error_msg) {
     load_signals_succeeded_ = success;
+    error_msg_ = std::move(error_msg);
+    EXPECT_EQ(load_signals_succeeded_, !error_msg_.has_value());
     load_signals_run_loop_->Quit();
   }
 
@@ -113,6 +116,7 @@ class TrustedBiddingSignalsTest : public testing::Test {
   // synchronously.
   std::unique_ptr<base::RunLoop> load_signals_run_loop_;
   bool load_signals_succeeded_ = false;
+  base::Optional<std::string> error_msg_;
 
   network::TestURLLoaderFactory url_loader_factory_;
   AuctionV8Helper v8_helper_;
@@ -123,18 +127,29 @@ TEST_F(TrustedBiddingSignalsTest, NetworkError) {
       "https://url.test/?hostname=publisher&keys=key1", kBaseJson,
       net::HTTP_NOT_FOUND);
   EXPECT_FALSE(FetchBiddingSignals({"key1"}, kHostname));
+  ASSERT_TRUE(error_msg_.has_value());
+  EXPECT_EQ(
+      "Failed to load https://url.test/?hostname=publisher&keys=key1 "
+      "HTTP status = 404 Not Found.",
+      error_msg_.value());
 }
 
 TEST_F(TrustedBiddingSignalsTest, ResponseNotJson) {
   EXPECT_FALSE(FetchBiddingSignalsWithResponse(
       GURL("https://url.test/?hostname=publisher&keys=key1"), "Not Json",
       {"key1"}, kHostname));
+  ASSERT_TRUE(error_msg_.has_value());
+  EXPECT_EQ("https://url.test/ Unable to parse as a JSON object.",
+            error_msg_.value());
 }
 
 TEST_F(TrustedBiddingSignalsTest, ResponseNotObject) {
   EXPECT_FALSE(FetchBiddingSignalsWithResponse(
       GURL("https://url.test/?hostname=publisher&keys=key1"), "42", {"key1"},
       kHostname));
+  ASSERT_TRUE(error_msg_.has_value());
+  EXPECT_EQ("https://url.test/ Unable to parse as a JSON object.",
+            error_msg_.value());
 }
 
 TEST_F(TrustedBiddingSignalsTest, KeyMissing) {

content/services/auction_worklet/worklet_loader.cc

@@ -38,17 +38,20 @@ WorkletLoader::WorkletLoader(
 
 WorkletLoader::~WorkletLoader() = default;
 
-void WorkletLoader::OnDownloadComplete(std::unique_ptr<std::string> body) {
+void WorkletLoader::OnDownloadComplete(std::unique_ptr<std::string> body,
+                                       base::Optional<std::string> error_msg) {
   DCHECK(load_worklet_callback_);
 
   auction_downloader_.reset();
 
   if (!body) {
-    std::move(load_worklet_callback_).Run(nullptr /* worklet_script */);
+    std::move(load_worklet_callback_)
+        .Run(nullptr /* worklet_script */, std::move(error_msg));
     return;
   }
 
   std::unique_ptr<v8::Global<v8::UnboundScript>> global_script;
+  DCHECK(!error_msg.has_value());
 
   // Need to release the isolate and context before invoking the callback, in
   // case the `v8_helper_` is destroyed.
@@ -57,13 +60,15 @@ void WorkletLoader::OnDownloadComplete(std::unique_ptr<std::string> body) {
     v8::Context::Scope context_scope(v8_helper_->scratch_context());
 
     v8::Local<v8::UnboundScript> local_script;
-    if (v8_helper_->Compile(*body, script_source_url_).ToLocal(&local_script)) {
+    if (v8_helper_->Compile(*body, script_source_url_, error_msg)
+            .ToLocal(&local_script)) {
       global_script = std::make_unique<v8::Global<v8::UnboundScript>>(
           v8_helper_->isolate(), local_script);
     }
   }
 
-  std::move(load_worklet_callback_).Run(std::move(global_script));
+  std::move(load_worklet_callback_)
+      .Run(std::move(global_script), std::move(error_msg));
 }
 
 }  // namespace auction_worklet

content/services/auction_worklet/worklet_loader.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/optional.h"
 #include "services/network/public/mojom/url_loader_factory.mojom-forward.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
@@ -25,11 +26,9 @@ class WorkletLoader {
   // On success, `worklet_script` is compiled script, not bound to any context.
   // It can be repeatedly bound to different contexts and executed, without
   // persisting any state.
-  //
-  // TODO(mmenke): Pass along some sort of error string on failure, to make
-  // debugging easier.
   using LoadWorkletCallback = base::OnceCallback<void(
-      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script)>;
+      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+      base::Optional<std::string> error_msg)>;
 
   // Starts loading the worklet script on construction. Callback will be invoked
   // asynchronously once the data has been fetched or an error has occurred.
@@ -43,7 +42,8 @@ class WorkletLoader {
   ~WorkletLoader();
 
  private:
-  void OnDownloadComplete(std::unique_ptr<std::string> body);
+  void OnDownloadComplete(std::unique_ptr<std::string> body,
+                          base::Optional<std::string> error_msg);
 
   const GURL script_source_url_;
   AuctionV8Helper* const v8_helper_;

content/services/auction_worklet/worklet_loader_unittest.cc

@@ -16,10 +16,14 @@
 #include "content/services/auction_worklet/worklet_test_util.h"
 #include "net/http/http_status_code.h"
 #include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
 
+using testing::HasSubstr;
+using testing::StartsWith;
+
 namespace auction_worklet {
 namespace {
 
@@ -34,11 +38,18 @@ class WorkletLoaderTest : public testing::Test {
   ~WorkletLoaderTest() override = default;
 
   void LoadWorkletCallback(
-      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script) {
+      std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
+      base::Optional<std::string> error_msg) {
     load_succeeded_ = !!worklet_script;
+    error_msg_ = std::move(error_msg);
+    EXPECT_EQ(load_succeeded_, !error_msg_.has_value());
     run_loop_.Quit();
   }
 
+  std::string last_error_msg() const {
+    return error_msg_.value_or("Not an error");
+  }
+
  protected:
   base::test::TaskEnvironment task_environment_;
 
@@ -47,6 +58,7 @@ class WorkletLoaderTest : public testing::Test {
   GURL url_ = GURL("https://foo.test/");
   base::RunLoop run_loop_;
   bool load_succeeded_ = false;
+  base::Optional<std::string> error_msg_;
 };
 
 TEST_F(WorkletLoaderTest, NetworkError) {
@@ -59,6 +71,8 @@ TEST_F(WorkletLoaderTest, NetworkError) {
                      base::Unretained(this)));
   run_loop_.Run();
   EXPECT_FALSE(load_succeeded_);
+  EXPECT_EQ("Failed to load https://foo.test/ HTTP status = 404 Not Found.",
+            last_error_msg());
 }
 
 TEST_F(WorkletLoaderTest, CompileError) {
@@ -69,6 +83,8 @@ TEST_F(WorkletLoaderTest, CompileError) {
                      base::Unretained(this)));
   run_loop_.Run();
   EXPECT_FALSE(load_succeeded_);
+  EXPECT_THAT(last_error_msg(), StartsWith("https://foo.test/:1 "));
+  EXPECT_THAT(last_error_msg(), HasSubstr("SyntaxError"));
 }
 
 TEST_F(WorkletLoaderTest, Success) {
@@ -92,9 +108,10 @@ TEST_F(WorkletLoaderTest, DeleteDuringCallbackSuccess) {
       std::make_unique<WorkletLoader>(
           &url_loader_factory_, url_, v8_helper.get(),
           base::BindLambdaForTesting(
-              [&](std::unique_ptr<v8::Global<v8::UnboundScript>>
+              [&](std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
-                      worklet_script) {
+                  base::Optional<std::string> error_msg) {
                 EXPECT_TRUE(worklet_script);
+                EXPECT_FALSE(error_msg.has_value());
                 worklet_script.reset();
                 worklet_loader.reset();
                 v8_helper.reset();
@@ -114,9 +131,13 @@ TEST_F(WorkletLoaderTest, DeleteDuringCallbackCompileError) {
       std::make_unique<WorkletLoader>(
           &url_loader_factory_, url_, v8_helper.get(),
           base::BindLambdaForTesting(
-              [&](std::unique_ptr<v8::Global<v8::UnboundScript>>
+              [&](std::unique_ptr<v8::Global<v8::UnboundScript>> worklet_script,
-                      worklet_script) {
+                  base::Optional<std::string> error_msg) {
                 EXPECT_FALSE(worklet_script);
+                ASSERT_TRUE(error_msg.has_value());
+                EXPECT_THAT(error_msg.value(),
+                            StartsWith("https://foo.test/:1 "));
+                EXPECT_THAT(error_msg.value(), HasSubstr("SyntaxError"));
                 worklet_loader.reset();
                 v8_helper.reset();
                 run_loop.Quit();