diff --git a/content/browser/devtools/devtools_instrumentation.cc b/content/browser/devtools/devtools_instrumentation.cc
index ba619ff51dbbc..d7e9033daa96f 100644
--- a/content/browser/devtools/devtools_instrumentation.cc
+++ b/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) {
diff --git a/content/browser/devtools/devtools_instrumentation.h b/content/browser/devtools/devtools_instrumentation.h
index dad7c6dd8142a..32ffbe01a07d4 100644
--- a/content/browser/devtools/devtools_instrumentation.h
+++ b/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);
diff --git a/content/browser/interest_group/ad_auction_service_impl.cc b/content/browser/interest_group/ad_auction_service_impl.cc
index 1a0d6fcbd1d94..6b64c3e7b3dda 100644
--- a/content/browser/interest_group/ad_auction_service_impl.cc
+++ b/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);
diff --git a/content/browser/interest_group/ad_auction_service_impl.h b/content/browser/interest_group/ad_auction_service_impl.h
index 2bf799c1ab72f..7e26a5d70bda9 100644
--- a/content/browser/interest_group/ad_auction_service_impl.h
+++ b/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
diff --git a/content/services/auction_worklet/auction_downloader.cc b/content/services/auction_worklet/auction_downloader.cc
index 7a721156c25ab..01ace1e677886 100644
--- a/content/services/auction_worklet/auction_downloader.cc
+++ b/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
diff --git a/content/services/auction_worklet/auction_downloader.h b/content/services/auction_worklet/auction_downloader.h
index 0ef694ed67fce..55c98737ae884 100644
--- a/content/services/auction_worklet/auction_downloader.h
+++ b/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_;
diff --git a/content/services/auction_worklet/auction_downloader_unittest.cc b/content/services/auction_worklet/auction_downloader_unittest.cc
index 02fd7f0235178..6abf337105f9d 100644
--- a/content/services/auction_worklet/auction_downloader_unittest.cc
+++ b/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
diff --git a/content/services/auction_worklet/auction_runner.cc b/content/services/auction_worklet/auction_runner.cc
index da5d9123c9d52..8aa3b2abdbdc7 100644
--- a/content/services/auction_worklet/auction_runner.cc
+++ b/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;
}
diff --git a/content/services/auction_worklet/auction_runner.h b/content/services/auction_worklet/auction_runner.h
index a2c85ae9aa5f8..f72a3ae9abea2 100644
--- a/content/services/auction_worklet/auction_runner.h
+++ b/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 OnTrustedSignalsLoaded(BidState* state, bool load_result);
+ void OnBidderScriptLoaded(BidState* state,
+ 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
diff --git a/content/services/auction_worklet/auction_runner_unittest.cc b/content/services/auction_worklet/auction_runner_unittest.cc
index 9b0042970dcca..f1157876f816e 100644
--- a/content/services/auction_worklet/auction_runner_unittest.cc
+++ b/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
diff --git a/content/services/auction_worklet/auction_v8_helper.cc b/content/services/auction_worklet/auction_v8_helper.cc
index 71dcf4df4b856..5bff8e4630de5 100644
--- a/content/services/auction_worklet/auction_v8_helper.cc
+++ b/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())
- PrintMessage(v8_isolate->GetCurrentContext(), try_catch.Message());
+ if (try_catch.HasCaught()) {
+ 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,
- v8::Local<v8::Message> message) {
+std::string AuctionV8Helper::FormatExceptionMessage(
+ 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())
- << (!context.IsEmpty() &&
- message->GetLineNumber(context).To(&line_num)
- ? std::string(":") + base::NumberToString(line_num)
- : std::string())
- << " " << FormatValue(isolate, message->Get());
+ return base::StrCat(
+ {FormatValue(isolate, message->GetScriptResourceName()),
+ !context.IsEmpty() && message->GetLineNumber(context).To(&line_num)
+ ? std::string(":") + base::NumberToString(line_num)
+ : std::string(),
+ " ", FormatValue(isolate, message->Get()), "."});
}
}
diff --git a/content/services/auction_worklet/auction_v8_helper.h b/content/services/auction_worklet/auction_v8_helper.h
index 663f237f12495..58882422733f7 100644
--- a/content/services/auction_worklet/auction_v8_helper.h
+++ b/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.
- v8::MaybeLocal<v8::UnboundScript> Compile(const std::string& src,
- const GURL& src_url);
+ // still must be an active context for this method to be invoked. In case of
+ // an error, sets `error_out`.
+ 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,
- v8::Local<v8::UnboundScript> script,
- base::StringPiece script_name,
- base::span<v8::Local<v8::Value>> args = {});
+ //
+ // In case of an error, sets `error_out`.
+ v8::MaybeLocal<v8::Value> RunScript(v8::Local<v8::Context> context,
+ v8::Local<v8::UnboundScript> script,
+ 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,
- v8::Local<v8::Message> message);
+ static std::string FormatExceptionMessage(v8::Local<v8::Context> context,
+ v8::Local<v8::Message> message);
static std::string FormatValue(v8::Isolate* isolate,
v8::Local<v8::Value> val);
diff --git a/content/services/auction_worklet/auction_v8_helper_unittest.cc b/content/services/auction_worklet/auction_v8_helper_unittest.cc
index 418d34ab16294..ae37ddb095ab3 100644
--- a/content/services/auction_worklet/auction_v8_helper_unittest.cc
+++ b/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(
- helper_
- .Compile("function foo() { return 1;}", GURL("https://foo.test/"))
- .ToLocal(&script));
+ 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());
}
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;}
- while(1);)",
+ {R"(function foo() { return 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);}
- while(1);)",
- };
+ {R"(function foo() {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 =
- helper_.RunScript(context, script, "foo");
+ v8::MaybeLocal<v8::Value> result = helper_.RunScript(
+ 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(
- helper_.Compile("function foo() { return 1;}", GURL("https://foo.test/"))
- .ToLocal(&script));
+ 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::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
diff --git a/content/services/auction_worklet/bidder_worklet.cc b/content/services/auction_worklet/bidder_worklet.cc
index c2f73b21db129..2846f2b5f17dc 100644
--- a/content/services/auction_worklet/bidder_worklet.cc
+++ b/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
- // created when there's no bidding URL.
+ // TODO(mmenke): Remove up the value_or() for script_source_url_- auction
+ // worklets shouldn't be created when there's no bidding URL.
worklet_loader_ = std::make_unique<WorkletLoader>(
- url_loader_factory,
- bidding_interest_group_->group->bidding_url.value_or(GURL()), v8_helper,
+ url_loader_factory, script_source_url_, 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)
- .ToLocal(&generate_bid_result) ||
- !generate_bid_result->IsObject()) {
- return BidResult();
+ args, error_msg_out)
+ .ToLocal(&generate_bid_result)) {
+ return BidResult(std::move(error_msg_out));
+ }
+
+ 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))
- return BidResult();
+ if (!render_url.is_valid() || !render_url.SchemeIs(url::kHttpsScheme)) {
+ 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
diff --git a/content/services/auction_worklet/bidder_worklet.h b/content/services/auction_worklet/bidder_worklet.h
index c1d3a9fce6847..85fec55ce7c28 100644
--- a/content/services/auction_worklet/bidder_worklet.h
+++ b/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_;
diff --git a/content/services/auction_worklet/bidder_worklet_unittest.cc b/content/services/auction_worklet/bidder_worklet_unittest.cc
index c18d6237d148d..99d9fd8f1327e 100644
--- a/content/services/auction_worklet/bidder_worklet_unittest.cc
+++ b/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());
- RunGenerateBidWithReturnValueExpectingResult(R"({ad: ["ad"], bid:"a"})",
- BidderWorklet::BidResult());
+ BidderWorklet::BidResult("https://url.test/ generateBid() return value "
+ "has incorrect structure."));
+ 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());
- RunGenerateBidWithJavascriptExpectingResult("", BidderWorklet::BidResult());
- RunGenerateBidWithJavascriptExpectingResult("5", BidderWorklet::BidResult());
- RunGenerateBidWithJavascriptExpectingResult("shrimp",
- BidderWorklet::BidResult());
+ BidderWorklet::BidResult(
+ "https://url.test/ `generateBid` is not a function."));
+ RunGenerateBidWithJavascriptExpectingResult(
+ "", 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",
- BidderWorklet::BidResult());
+ RunGenerateBidWithJavascriptExpectingResult(
+ "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) {
- signals_loaded_successfully = success;
- run_loop.Quit();
- }));
+ base::BindLambdaForTesting(
+ [&](bool success, base::Optional<std::string> signals_error_msg) {
+ 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();
}
diff --git a/content/services/auction_worklet/public/mojom/auction_worklet_service.mojom b/content/services/auction_worklet/public/mojom/auction_worklet_service.mojom
index 6552c606b9c7d..2b0adb60137c0 100644
--- a/content/services/auction_worklet/public/mojom/auction_worklet_service.mojom
+++ b/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);
};
diff --git a/content/services/auction_worklet/report_bindings.cc b/content/services/auction_worklet/report_bindings.cc
index 4f9f98c5c9670..d7c5ab0de2512 100644
--- a/content/services/auction_worklet/report_bindings.cc
+++ b/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;
}
diff --git a/content/services/auction_worklet/seller_worklet.cc b/content/services/auction_worklet/seller_worklet.cc
index fc36a677f9c47..750c27223af3e 100644
--- a/content/services/auction_worklet/seller_worklet.cc
+++ b/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)
- .ToLocal(&score_ad_result) ||
- !gin::ConvertFromV8(isolate, score_ad_result, &score)) {
- return ScoreResult();
+ ->RunScript(context, worklet_script_->Get(isolate), "scoreAd", args,
+ error_msg_out)
+ .ToLocal(&score_ad_result)) {
+ 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
diff --git a/content/services/auction_worklet/seller_worklet.h b/content/services/auction_worklet/seller_worklet.h
index 3ce787c941f7f..cb1043935531c 100644
--- a/content/services/auction_worklet/seller_worklet.h
+++ b/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
- // to add error information down the line.
+ // The result of invoking scoreAd().
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_;
diff --git a/content/services/auction_worklet/seller_worklet_unittest.cc b/content/services/auction_worklet/seller_worklet_unittest.cc
index 58c411e9ca247..bdaca9acadd5c 100644
--- a/content/services/auction_worklet/seller_worklet_unittest.cc
+++ b/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,
- double expected_score) {
+ void RunScoreAdWithJavascriptExpectingResult(
+ 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("1/0", 0);
- RunScoreAdWithReturnValueExpectingResult("0/0", 0);
- RunScoreAdWithReturnValueExpectingResult("-1/0", 0);
- RunScoreAdWithReturnValueExpectingResult("true", 0);
+ RunScoreAdWithReturnValueExpectingResult(
+ "[15]", 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(
+ "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"))",
- SellerWorklet::Report());
- RunReportResultCreatedScriptExpectingResult("1", R"(sendReportTo(null))",
- SellerWorklet::Report());
- RunReportResultCreatedScriptExpectingResult("1", R"(sendReportTo([5]))",
- SellerWorklet::Report());
+ RunReportResultCreatedScriptExpectingResult(
+ "1", R"(sendReportTo("France"))",
+ SellerWorklet::Report("https://url.test/:3 Uncaught TypeError: "
+ "sendReportTo must be passed a valid HTTPS url."));
+ RunReportResultCreatedScriptExpectingResult(
+ "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) {
diff --git a/content/services/auction_worklet/trusted_bidding_signals.cc b/content/services/auction_worklet/trusted_bidding_signals.cc
index f9da2b48edb66..894c91d727820 100644
--- a/content/services/auction_worklet/trusted_bidding_signals.cc
+++ b/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
diff --git a/content/services/auction_worklet/trusted_bidding_signals.h b/content/services/auction_worklet/trusted_bidding_signals.h
index 59e7b2a6b99d5..f0adee6b9fee8 100644
--- a/content/services/auction_worklet/trusted_bidding_signals.h
+++ b/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_;
diff --git a/content/services/auction_worklet/trusted_bidding_signals_unittest.cc b/content/services/auction_worklet/trusted_bidding_signals_unittest.cc
index 5273f466e3512..984857683c442 100644
--- a/content/services/auction_worklet/trusted_bidding_signals_unittest.cc
+++ b/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) {
diff --git a/content/services/auction_worklet/worklet_loader.cc b/content/services/auction_worklet/worklet_loader.cc
index 689372ea6d04c..b2aa1ee4bff3b 100644
--- a/content/services/auction_worklet/worklet_loader.cc
+++ b/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
diff --git a/content/services/auction_worklet/worklet_loader.h b/content/services/auction_worklet/worklet_loader.h
index 47136682ed154..dfa081773f0b9 100644
--- a/content/services/auction_worklet/worklet_loader.h
+++ b/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_;
diff --git a/content/services/auction_worklet/worklet_loader_unittest.cc b/content/services/auction_worklet/worklet_loader_unittest.cc
index b42a666c2f072..25e1c6718720a 100644
--- a/content/services/auction_worklet/worklet_loader_unittest.cc
+++ b/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>>
- worklet_script) {
+ [&](std::unique_ptr<v8::Global<v8::UnboundScript>> 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>>
- worklet_script) {
+ [&](std::unique_ptr<v8::Global<v8::UnboundScript>> 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();