Merge pull request #87628 from YuriSizov/assets-bigger-better-errors

Improve error reporting in the asset library and in related types

core/io/image.cpp

@@ -2781,7 +2781,7 @@ void Image::_get_clipped_src_and_dest_rects(const Ref<Image> &p_src, const Rect2
 }
 
 void Image::blit_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest) {
-	ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
+	ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blit_rect an image: invalid source Image object.");
 	int dsize = data.size();
 	int srcdsize = p_src->data.size();
 	ERR_FAIL_COND(dsize == 0);
@@ -2823,8 +2823,8 @@ void Image::blit_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const P
 }
 
 void Image::blit_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, const Rect2i &p_src_rect, const Point2i &p_dest) {
-	ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
-	ERR_FAIL_COND_MSG(p_mask.is_null(), "It's not a reference to a valid Image object.");
+	ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blit_rect_mask an image: invalid source Image object.");
+	ERR_FAIL_COND_MSG(p_mask.is_null(), "Cannot blit_rect_mask an image: invalid mask Image object.");
 	int dsize = data.size();
 	int srcdsize = p_src->data.size();
 	int maskdsize = p_mask->data.size();
@@ -2873,7 +2873,7 @@ void Image::blit_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, co
 }
 
 void Image::blend_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest) {
-	ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
+	ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blend_rect an image: invalid source Image object.");
 	int dsize = data.size();
 	int srcdsize = p_src->data.size();
 	ERR_FAIL_COND(dsize == 0);
@@ -2908,8 +2908,8 @@ void Image::blend_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const
 }
 
 void Image::blend_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, const Rect2i &p_src_rect, const Point2i &p_dest) {
-	ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
-	ERR_FAIL_COND_MSG(p_mask.is_null(), "It's not a reference to a valid Image object.");
+	ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blend_rect_mask an image: invalid source Image object.");
+	ERR_FAIL_COND_MSG(p_mask.is_null(), "Cannot blend_rect_mask an image: invalid mask Image object.");
 	int dsize = data.size();
 	int srcdsize = p_src->data.size();
 	int maskdsize = p_mask->data.size();

core/io/image.h

@@ -431,7 +431,7 @@ public:
 	void set_as_black();
 
 	void copy_internals_from(const Ref<Image> &p_image) {
-		ERR_FAIL_COND_MSG(p_image.is_null(), "It's not a reference to a valid Image object.");
+		ERR_FAIL_COND_MSG(p_image.is_null(), "Cannot copy image internals: invalid Image object.");
 		format = p_image->format;
 		width = p_image->width;
 		height = p_image->height;

core/io/image_loader.cpp

@@ -81,7 +81,7 @@ void ImageFormatLoaderExtension::_bind_methods() {
 }
 
 Error ImageLoader::load_image(String p_file, Ref<Image> p_image, Ref<FileAccess> p_custom, BitField<ImageFormatLoader::LoaderFlags> p_flags, float p_scale) {
-	ERR_FAIL_COND_V_MSG(p_image.is_null(), ERR_INVALID_PARAMETER, "It's not a reference to a valid Image object.");
+	ERR_FAIL_COND_V_MSG(p_image.is_null(), ERR_INVALID_PARAMETER, "Can't load an image: invalid Image object.");
 
 	Ref<FileAccess> f = p_custom;
 	if (f.is_null()) {

editor/plugins/asset_library_editor_plugin.cpp

@@ -758,86 +758,97 @@ void EditorAssetLibrary::_select_asset(int p_id) {
 	_api_request("asset/" + itos(p_id), REQUESTING_ASSET);
 }
 
-void EditorAssetLibrary::_image_update(bool use_cache, bool final, const PackedByteArray &p_data, int p_queue_id) {
+void EditorAssetLibrary::_image_update(bool p_use_cache, bool p_final, const PackedByteArray &p_data, int p_queue_id) {
 	Object *obj = ObjectDB::get_instance(image_queue[p_queue_id].target);
+	if (!obj) {
+		return;
+	}
 
-	if (obj) {
-		bool image_set = false;
-		PackedByteArray image_data = p_data;
+	bool image_set = false;
+	PackedByteArray image_data = p_data;
 
-		if (use_cache) {
-			String cache_filename_base = EditorPaths::get_singleton()->get_cache_dir().path_join("assetimage_" + image_queue[p_queue_id].image_url.md5_text());
+	if (p_use_cache) {
+		String cache_filename_base = EditorPaths::get_singleton()->get_cache_dir().path_join("assetimage_" + image_queue[p_queue_id].image_url.md5_text());
 
-			Ref<FileAccess> file = FileAccess::open(cache_filename_base + ".data", FileAccess::READ);
-			if (file.is_valid()) {
-				PackedByteArray cached_data;
-				int len = file->get_32();
-				cached_data.resize(len);
+		Ref<FileAccess> file = FileAccess::open(cache_filename_base + ".data", FileAccess::READ);
+		if (file.is_valid()) {
+			PackedByteArray cached_data;
+			int len = file->get_32();
+			cached_data.resize(len);
 
-				uint8_t *w = cached_data.ptrw();
-				file->get_buffer(w, len);
+			uint8_t *w = cached_data.ptrw();
+			file->get_buffer(w, len);
 
-				image_data = cached_data;
-			}
+			image_data = cached_data;
+		}
+	}
+
+	int len = image_data.size();
+	const uint8_t *r = image_data.ptr();
+	Ref<Image> image = memnew(Image);
+
+	uint8_t png_signature[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
+	uint8_t jpg_signature[3] = { 255, 216, 255 };
+	uint8_t webp_signature[4] = { 82, 73, 70, 70 };
+	uint8_t bmp_signature[2] = { 66, 77 };
+
+	if (r) {
+		Ref<Image> parsed_image;
+
+		if ((memcmp(&r[0], &png_signature[0], 8) == 0) && Image::_png_mem_loader_func) {
+			parsed_image = Image::_png_mem_loader_func(r, len);
+		} else if ((memcmp(&r[0], &jpg_signature[0], 3) == 0) && Image::_jpg_mem_loader_func) {
+			parsed_image = Image::_jpg_mem_loader_func(r, len);
+		} else if ((memcmp(&r[0], &webp_signature[0], 4) == 0) && Image::_webp_mem_loader_func) {
+			parsed_image = Image::_webp_mem_loader_func(r, len);
+		} else if ((memcmp(&r[0], &bmp_signature[0], 2) == 0) && Image::_bmp_mem_loader_func) {
+			parsed_image = Image::_bmp_mem_loader_func(r, len);
+		} else if (Image::_svg_scalable_mem_loader_func) {
+			parsed_image = Image::_svg_scalable_mem_loader_func(r, len, 1.0);
 		}
 
-		int len = image_data.size();
-		const uint8_t *r = image_data.ptr();
-		Ref<Image> image = Ref<Image>(memnew(Image));
-
-		uint8_t png_signature[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
-		uint8_t jpg_signature[3] = { 255, 216, 255 };
-		uint8_t webp_signature[4] = { 82, 73, 70, 70 };
-		uint8_t bmp_signature[2] = { 66, 77 };
-
-		if (r) {
-			if ((memcmp(&r[0], &png_signature[0], 8) == 0) && Image::_png_mem_loader_func) {
-				image->copy_internals_from(Image::_png_mem_loader_func(r, len));
-			} else if ((memcmp(&r[0], &jpg_signature[0], 3) == 0) && Image::_jpg_mem_loader_func) {
-				image->copy_internals_from(Image::_jpg_mem_loader_func(r, len));
-			} else if ((memcmp(&r[0], &webp_signature[0], 4) == 0) && Image::_webp_mem_loader_func) {
-				image->copy_internals_from(Image::_webp_mem_loader_func(r, len));
-			} else if ((memcmp(&r[0], &bmp_signature[0], 2) == 0) && Image::_bmp_mem_loader_func) {
-				image->copy_internals_from(Image::_bmp_mem_loader_func(r, len));
-			} else if (Image::_svg_scalable_mem_loader_func) {
-				image->copy_internals_from(Image::_svg_scalable_mem_loader_func(r, len, 1.0));
+		if (parsed_image.is_null()) {
+			if (is_print_verbose_enabled()) {
+				ERR_PRINT(vformat("Asset Library: Invalid image downloaded from '%s' for asset # %d", image_queue[p_queue_id].image_url, image_queue[p_queue_id].asset_id));
 			}
+		} else {
+			image->copy_internals_from(parsed_image);
 		}
+	}
 
-		if (!image->is_empty()) {
-			switch (image_queue[p_queue_id].image_type) {
-				case IMAGE_QUEUE_ICON:
+	if (!image->is_empty()) {
+		switch (image_queue[p_queue_id].image_type) {
+			case IMAGE_QUEUE_ICON:
+				image->resize(64 * EDSCALE, 64 * EDSCALE, Image::INTERPOLATE_LANCZOS);
+				break;
 
-					image->resize(64 * EDSCALE, 64 * EDSCALE, Image::INTERPOLATE_LANCZOS);
+			case IMAGE_QUEUE_THUMBNAIL: {
+				float max_height = 85 * EDSCALE;
 
-					break;
-				case IMAGE_QUEUE_THUMBNAIL: {
-					float max_height = 85 * EDSCALE;
+				float scale_ratio = max_height / (image->get_height() * EDSCALE);
+				if (scale_ratio < 1) {
+					image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
+				}
+			} break;
 
-					float scale_ratio = max_height / (image->get_height() * EDSCALE);
-					if (scale_ratio < 1) {
-						image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
-					}
-				} break;
-				case IMAGE_QUEUE_SCREENSHOT: {
-					float max_height = 397 * EDSCALE;
+			case IMAGE_QUEUE_SCREENSHOT: {
+				float max_height = 397 * EDSCALE;
 
-					float scale_ratio = max_height / (image->get_height() * EDSCALE);
-					if (scale_ratio < 1) {
-						image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
-					}
-				} break;
-			}
+				float scale_ratio = max_height / (image->get_height() * EDSCALE);
+				if (scale_ratio < 1) {
+					image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
+				}
+			} break;
+		}
 
-			Ref<ImageTexture> tex = ImageTexture::create_from_image(image);
+		Ref<ImageTexture> tex = ImageTexture::create_from_image(image);
 
-			obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, tex);
-			image_set = true;
-		}
+		obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, tex);
+		image_set = true;
+	}
 
-		if (!image_set && final) {
-			obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
-		}
+	if (!image_set && p_final) {
+		obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
 	}
 }
 
@@ -870,7 +881,10 @@ void EditorAssetLibrary::_image_request_completed(int p_status, int p_code, cons
 		_image_update(p_code == HTTPClient::RESPONSE_NOT_MODIFIED, true, p_data, p_queue_id);
 
 	} else {
-		WARN_PRINT("Error getting image file from URL: " + image_queue[p_queue_id].image_url);
+		if (is_print_verbose_enabled()) {
+			WARN_PRINT(vformat("Asset Library: Error getting image from '%s' for asset # %d.", image_queue[p_queue_id].image_url, image_queue[p_queue_id].asset_id));
+		}
+
 		Object *obj = ObjectDB::get_instance(image_queue[p_queue_id].target);
 		if (obj) {
 			obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
@@ -919,22 +933,48 @@ void EditorAssetLibrary::_update_image_queue() {
 	}
 }
 
-void EditorAssetLibrary::_request_image(ObjectID p_for, String p_image_url, ImageType p_type, int p_image_index) {
+void EditorAssetLibrary::_request_image(ObjectID p_for, int p_asset_id, String p_image_url, ImageType p_type, int p_image_index) {
+	// Remove extra spaces around the URL. This isn't strictly valid, but recoverable.
+	String trimmed_url = p_image_url.strip_edges();
+	if (trimmed_url != p_image_url && is_print_verbose_enabled()) {
+		WARN_PRINT(vformat("Asset Library: Badly formatted image URL '%s' for asset # %d.", p_image_url, p_asset_id));
+	}
+
+	// Validate the image URL first.
+	{
+		String url_scheme;
+		String url_host;
+		int url_port;
+		String url_path;
+		Error err = trimmed_url.parse_url(url_scheme, url_host, url_port, url_path);
+		if (err != OK) {
+			if (is_print_verbose_enabled()) {
+				ERR_PRINT(vformat("Asset Library: Invalid image URL '%s' for asset # %d.", trimmed_url, p_asset_id));
+			}
+
+			Object *obj = ObjectDB::get_instance(p_for);
+			if (obj) {
+				obj->call("set_image", p_type, p_image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
+			}
+			return;
+		}
+	}
+
 	ImageQueue iq;
-	iq.image_url = p_image_url;
+	iq.image_url = trimmed_url;
 	iq.image_index = p_image_index;
 	iq.image_type = p_type;
 	iq.request = memnew(HTTPRequest);
 	setup_http_request(iq.request);
 
 	iq.target = p_for;
+	iq.asset_id = p_asset_id;
 	iq.queue_id = ++last_queue_id;
 	iq.active = false;
 
 	iq.request->connect("request_completed", callable_mp(this, &EditorAssetLibrary::_image_request_completed).bind(iq.queue_id));
 
 	image_queue[iq.queue_id] = iq;
-
 	add_child(iq.request);
 
 	_image_update(true, false, PackedByteArray(), iq.queue_id);
@@ -1311,7 +1351,7 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
 				item->connect("category_selected", callable_mp(this, &EditorAssetLibrary::_select_category));
 
 				if (r.has("icon_url") && !r["icon_url"].operator String().is_empty()) {
-					_request_image(item->get_instance_id(), r["icon_url"], IMAGE_QUEUE_ICON, 0);
+					_request_image(item->get_instance_id(), r["asset_id"], r["icon_url"], IMAGE_QUEUE_ICON, 0);
 				}
 			}
 
@@ -1362,7 +1402,7 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
 			}
 
 			if (r.has("icon_url") && !r["icon_url"].operator String().is_empty()) {
-				_request_image(description->get_instance_id(), r["icon_url"], IMAGE_QUEUE_ICON, 0);
+				_request_image(description->get_instance_id(), r["asset_id"], r["icon_url"], IMAGE_QUEUE_ICON, 0);
 			}
 
 			if (d.has("previews")) {
@@ -1383,11 +1423,11 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
 					description->add_preview(i, is_video, video_url);
 
 					if (p.has("thumbnail")) {
-						_request_image(description->get_instance_id(), p["thumbnail"], IMAGE_QUEUE_THUMBNAIL, i);
+						_request_image(description->get_instance_id(), r["asset_id"], p["thumbnail"], IMAGE_QUEUE_THUMBNAIL, i);
 					}
 
 					if (!is_video) {
-						_request_image(description->get_instance_id(), p["link"], IMAGE_QUEUE_SCREENSHOT, i);
+						_request_image(description->get_instance_id(), r["asset_id"], p["link"], IMAGE_QUEUE_SCREENSHOT, i);
 					}
 				}
 			}

editor/plugins/asset_library_editor_plugin.h

@@ -262,14 +262,15 @@ class EditorAssetLibrary : public PanelContainer {
 		String image_url;
 		HTTPRequest *request = nullptr;
 		ObjectID target;
+		int asset_id = -1;
 	};
 
 	int last_queue_id;
 	HashMap<int, ImageQueue> image_queue;
 
-	void _image_update(bool use_cache, bool final, const PackedByteArray &p_data, int p_queue_id);
+	void _image_update(bool p_use_cache, bool p_final, const PackedByteArray &p_data, int p_queue_id);
 	void _image_request_completed(int p_status, int p_code, const PackedStringArray &headers, const PackedByteArray &p_data, int p_queue_id);
-	void _request_image(ObjectID p_for, String p_image_url, ImageType p_type, int p_image_index);
+	void _request_image(ObjectID p_for, int p_asset_id, String p_image_url, ImageType p_type, int p_image_index);
 	void _update_image_queue();
 
 	HBoxContainer *_make_pages(int p_page, int p_page_count, int p_page_len, int p_total_items, int p_current_items);

modules/svg/image_loader_svg.cpp

@@ -72,7 +72,7 @@ Ref<Image> ImageLoaderSVG::load_mem_svg(const uint8_t *p_svg, int p_size, float
 	img.instantiate();
 
 	Error err = create_image_from_utf8_buffer(img, p_svg, p_size, p_scale, false);
-	ERR_FAIL_COND_V(err, Ref<Image>());
+	ERR_FAIL_COND_V_MSG(err != OK, Ref<Image>(), vformat("ImageLoaderSVG: Failed to create SVG from buffer, error code %d.", err));
 
 	return img;
 }

scene/main/http_request.cpp

@@ -50,12 +50,14 @@ Error HTTPRequest::_parse_url(const String &p_url) {
 
 	String scheme;
 	Error err = p_url.parse_url(scheme, url, port, request_string);
-	ERR_FAIL_COND_V_MSG(err != OK, err, "Error parsing URL: " + p_url + ".");
+	ERR_FAIL_COND_V_MSG(err != OK, err, vformat("Error parsing URL: '%s'.", p_url));
+
 	if (scheme == "https://") {
 		use_tls = true;
 	} else if (scheme != "http://") {
-		ERR_FAIL_V_MSG(ERR_INVALID_PARAMETER, "Invalid URL scheme: " + scheme + ".");
+		ERR_FAIL_V_MSG(ERR_INVALID_PARAMETER, vformat("Invalid URL scheme: '%s'.", scheme));
 	}
+
 	if (port == 0) {
 		port = use_tls ? 443 : 80;
 	}