From 65adc96a211085b9b012b90629c57a110836f315 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 11 Mar 2025 18:56:28 +0100 Subject: [PATCH 01/21] drafts ideas --- .../message_extraction.py | 59 ++++++++++++ .../message_refinement.py | 90 +++++++++++++++++++ .../message_reintegration.py | 43 +++++++++ 3 files changed, 192 insertions(+) create mode 100644 scripts/user-message-reviewer/message_extraction.py create mode 100644 scripts/user-message-reviewer/message_refinement.py create mode 100644 scripts/user-message-reviewer/message_reintegration.py diff --git a/scripts/user-message-reviewer/message_extraction.py b/scripts/user-message-reviewer/message_extraction.py new file mode 100644 index 000000000000..26e7aa261292 --- /dev/null +++ b/scripts/user-message-reviewer/message_extraction.py @@ -0,0 +1,59 @@ +import ast +import json +from pathlib import Path + +""" +def mark_for_review(message): + return message + +# Example error messages +error_message = mark_for_review("Invalid input: '{input_value}'") +not_found_message = mark_for_review("File '{filename}' was not found") +""" + + +def extract_unreviewed_messages(code_file, output_file, condition=None): + """ + Extracts messages from mark_for_review calls that match a condition on the version tag. + + Parameters: + code_file (str): Path to the Python source code file. + output_file (str): Path to save the extracted messages as a JSON file. + condition (callable, optional): A function to evaluate the version tag. If None, extracts messages with no version tag. + """ + with Path.open(code_file) as f: + tree = ast.parse(f.read()) + + messages = {} + + # Walk through the AST to find calls to `mark_for_review` + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and getattr(node.func, "id", None) == "mark_for_review" + ): + if len(node.args) >= 1 and isinstance(node.args[0], ast.Constant): + original_message = node.args[0].value + version = None + + # Check if a second argument (version) exists + if len(node.args) > 1 and isinstance(node.args[1], ast.Constant): + version = node.args[1].value + + # Apply the filter condition + if condition is None or condition(version): + key = f"{node.lineno}_{node.col_offset}" # Unique key based on location + messages[key] = {"message": original_message, "version": version} + + # Save extracted messages to a JSON file + with Path.open(output_file, "w") as f: + json.dump(messages, f, indent=4) + + +# Example usage +# Condition: Extract messages with no version or explicitly unreviewed (e.g., version is None or "unreviewed") +def is_unreviewed(version): + return version is None or version == "unreviewed" + + +# extract_unreviewed_messages("your_script.py", "unreviewed_messages.json", condition=is_unreviewed) diff --git a/scripts/user-message-reviewer/message_refinement.py b/scripts/user-message-reviewer/message_refinement.py new file mode 100644 index 000000000000..cbfd58fd386c --- /dev/null +++ b/scripts/user-message-reviewer/message_refinement.py @@ -0,0 +1,90 @@ +import json +import os +from pathlib import Path + +import openai + +# Set your OpenAI API key +openai.api_key = os.environ["OPENAPI_API_KEY"] + + +def load_rules(rules_file): + """Load rules for message refinement from a JSON file.""" + with Path.open(rules_file) as f: + return json.load(f) + + +def load_messages(messages_file): + """Load messages from the messages.json file.""" + with Path.open(messages_file) as f: + return json.load(f) + + +def send_to_chatgpt(messages, rules): + """Send messages and rules to ChatGPT and request alternatives.""" + refined_messages = {} + for key, details in messages.items(): + message = details["message"] + version = details.get("version", "unknown") + + # Prepare the prompt + prompt = f""" +You are a language expert. Apply the following rules to suggest alternatives for the given message: + +Rules: +- Tone: {rules['tone']} +- Style: {rules['style']} +- Length: {rules['length']} +- Placeholders: {rules['placeholders']} + +Original Message: "{message}" + +Provide three alternatives based on the above rules. Ensure placeholders remain intact. +""" + + # Send the request to ChatGPT + response = openai.Completion.create( + engine="text-davinci-003", prompt=prompt, max_tokens=150, temperature=0.7 + ) + + # Parse the response + alternatives = response.choices[0].text.strip() + refined_messages[key] = { + "original": message, + "version": version, + "alternatives": alternatives.split( + "\n" + ), # Split into list if alternatives are on separate lines + } + + return refined_messages + + +def save_refined_messages(output_file, refined_messages): + """Save the refined messages to a JSON file.""" + with Path.open(output_file, "w") as f: + json.dump(refined_messages, f, indent=4) + + +# Example usage +# update_messages_in_code("your_script.py", "messages.json") + + +# Main script +if __name__ == "__main__": + # File paths + messages_file = "messages.json" + rules_file = "rules.json" + output_file = "refined_messages.json" + + # Load inputs + rules = load_rules(rules_file) + messages = load_messages(messages_file) + + # Process messages with ChatGPT + refined_messages = send_to_chatgpt(messages, rules) + + # Save the refined messages + save_refined_messages(output_file, refined_messages) + + print(f"Refined messages saved to {output_file}.") diff --git a/scripts/user-message-reviewer/message_reintegration.py b/scripts/user-message-reviewer/message_reintegration.py new file mode 100644 index 000000000000..cea9aed8624d --- /dev/null +++ b/scripts/user-message-reviewer/message_reintegration.py @@ -0,0 +1,43 @@ +import ast +import json +from pathlib import Path + +import astor + + +def update_messages_in_code(code_file, messages_file): + # Load corrected messages from the JSON file + with Path.open(messages_file) as f: + corrected_messages = json.load(f) + + # Parse the Python code + with Path.open(code_file) as f: + tree = ast.parse(f.read()) + + # Walk through the AST to find calls to `mark_for_review` + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and getattr(node.func, "id", None) == "mark_for_review" + ): + # Get the original message + if isinstance(node.args[0], ast.Constant): + original_message = node.args[0].value + + # Match it with corrected messages + for key, corrected_message in corrected_messages.items(): + if original_message == corrected_message["original"]: + # Replace the message with the corrected version + node.args[0].value = corrected_message["current"] + + # Add the version as the second argument + version_node = ast.Constant(value=corrected_message["version"]) + if len(node.args) == 1: + node.args.append(version_node) + else: + node.args[1] = version_node + + # Write the updated code back to the file + updated_code = astor.to_source(tree) + with Path.open(code_file, "w") as f: + f.write(updated_code) From 7a7746ff2b4a04419d14727f2c0347cfe5477689 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 13 Mar 2025 20:20:44 +0100 Subject: [PATCH 02/21] implements #7362 --- .../src/common_library/error_codes.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/common-library/src/common_library/error_codes.py b/packages/common-library/src/common_library/error_codes.py index 70829a059ca3..0043ffac0f8f 100644 --- a/packages/common-library/src/common_library/error_codes.py +++ b/packages/common-library/src/common_library/error_codes.py @@ -51,6 +51,19 @@ def _create_timestamp() -> int: ts = datetime.now(UTC).timestamp() * _SECS_TO_MILISECS return int(ts) +_LEN = 12 # chars (~48 bits) + + +def _generate_error_fingerprint(exc: BaseException) -> str: + """ + Unique error fingerprint for deduplication purposes + """ + tb = traceback.extract_tb(exc.__traceback__) + frame_sigs = [f"{frame.name}:{frame.lineno}" for frame in tb] + fingerprint = f"{type(exc).__name__}|" + "|".join(frame_sigs) + # E.g. ZeroDivisionError|foo:23|main:10 + return hashlib.sha256(fingerprint.encode()).hexdigest()[:_LEN] + def create_error_code(exception: BaseException) -> ErrorCodeStr: """ From f32d961e83a9b1af73c1a899ddc976345dd59c27 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 19 Mar 2025 12:25:17 +0100 Subject: [PATCH 03/21] reverts bad merge --- .../src/common_library/error_codes.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/common-library/src/common_library/error_codes.py b/packages/common-library/src/common_library/error_codes.py index 0043ffac0f8f..70829a059ca3 100644 --- a/packages/common-library/src/common_library/error_codes.py +++ b/packages/common-library/src/common_library/error_codes.py @@ -51,19 +51,6 @@ def _create_timestamp() -> int: ts = datetime.now(UTC).timestamp() * _SECS_TO_MILISECS return int(ts) -_LEN = 12 # chars (~48 bits) - - -def _generate_error_fingerprint(exc: BaseException) -> str: - """ - Unique error fingerprint for deduplication purposes - """ - tb = traceback.extract_tb(exc.__traceback__) - frame_sigs = [f"{frame.name}:{frame.lineno}" for frame in tb] - fingerprint = f"{type(exc).__name__}|" + "|".join(frame_sigs) - # E.g. ZeroDivisionError|foo:23|main:10 - return hashlib.sha256(fingerprint.encode()).hexdigest()[:_LEN] - def create_error_code(exception: BaseException) -> ErrorCodeStr: """ From 5e6c7634704516f46300ab70ead8c3b5891a9635 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 28 Mar 2025 23:16:07 +0100 Subject: [PATCH 04/21] errors templates --- .../src/common_library/errors_classes.py | 44 ++++++++++++++++ .../tests/test_errors_classes.py | 51 ++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/packages/common-library/src/common_library/errors_classes.py b/packages/common-library/src/common_library/errors_classes.py index dfee557d38ce..00cae26d1b25 100644 --- a/packages/common-library/src/common_library/errors_classes.py +++ b/packages/common-library/src/common_library/errors_classes.py @@ -52,3 +52,47 @@ def error_context(self) -> dict[str, Any]: def error_code(self) -> str: assert isinstance(self, Exception), "subclass must be exception" # nosec return create_error_code(self) + + +class BaseOsparcError(OsparcErrorMixin, Exception): ... + + +class NotFoundError(BaseOsparcError): + msg_template = "{resource} not found: id='{resource_id}'" + + +class ForbiddenError(BaseOsparcError): + msg_template = "Access to {resource} is forbidden: id='{resource_id}'" + + +def make_resource_error( + resource: str, + error_cls: type[BaseOsparcError], + base_exception: type[Exception] = Exception, +) -> type[BaseOsparcError]: + """ + Factory function to create a custom error class for a specific resource. + + This function dynamically generates an error class that inherits from the provided + `error_cls` and optionally a `base_exception`. The generated error class automatically + includes the resource name and resource ID in its context and message. + + See usage examples in test_errors_classes.py + + LIMITATIONS: for the moment, exceptions produces with this factory cannot be serialized with pickle. + And therefore it cannot be used as exception of RabbitMQ-RPC interface + """ + + class _ResourceError(error_cls, base_exception): + def __init__(self, **ctx: Any): + ctx.setdefault("resource", resource) + + # guesses identifer e.g. project_id, user_id + if resource_id := ctx.get(f"{resource.lower()}_id"): + ctx.setdefault("resource_id", resource_id) + + super().__init__(**ctx) + + resource_class_name = "".join(word.capitalize() for word in resource.split("_")) + _ResourceError.__name__ = f"{resource_class_name}{error_cls.__name__}" + return _ResourceError diff --git a/packages/common-library/tests/test_errors_classes.py b/packages/common-library/tests/test_errors_classes.py index 808ed09c40dd..5a6c4d5f87ad 100644 --- a/packages/common-library/tests/test_errors_classes.py +++ b/packages/common-library/tests/test_errors_classes.py @@ -9,7 +9,12 @@ from typing import Any import pytest -from common_library.errors_classes import OsparcErrorMixin +from common_library.errors_classes import ( + ForbiddenError, + NotFoundError, + OsparcErrorMixin, + make_resource_error, +) def test_get_full_class_name(): @@ -154,3 +159,47 @@ class MyError(OsparcErrorMixin, ValueError): "message": "42 and 'missing=?'", "value": 42, } + + +def test_resource_error_factory(): + ProjectNotFoundError = make_resource_error("project", NotFoundError) + + error_1 = ProjectNotFoundError(resource_id="abc123") + assert "resource_id" in error_1.error_context() + assert error_1.resource_id in error_1.message # type: ignore + + +def test_resource_error_factory_auto_detect_resource_id(): + ProjectForbiddenError = make_resource_error("project", ForbiddenError) + error_2 = ProjectForbiddenError(project_id="abc123", other_id="foo") + assert ( + error_2.resource_id == error_2.project_id # type: ignore + ), "auto-detects project ids as resourceid" + assert error_2.other_id # type: ignore + assert error_2.code == "BaseOsparcError.ForbiddenError.ProjectForbiddenError" + + assert error_2.error_context() == { + "project_id": "abc123", + "other_id": "foo", + "resource": "project", + "resource_id": "abc123", + "message": "Access to project is forbidden: id='abc123'", + "code": "BaseOsparcError.ForbiddenError.ProjectForbiddenError", + } + + +def test_resource_error_factory_different_base_exception(): + + class MyServiceError(Exception): ... + + OtherProjectForbiddenError = make_resource_error( + "other_project", ForbiddenError, MyServiceError + ) + + assert issubclass(OtherProjectForbiddenError, MyServiceError) + + error_3 = OtherProjectForbiddenError(project_id="abc123") + assert ( + error_3.code + == "MyServiceError.BaseOsparcError.ForbiddenError.OtherProjectForbiddenError" + ) From 415ab4731b7c85850f11742b6bfdff66f330d91f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 9 Apr 2025 15:24:28 +0200 Subject: [PATCH 05/21] feat: Add script for rewriting error messages using OpenAI API --- scripts/rewrite_errors_llm.py | 127 ++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 scripts/rewrite_errors_llm.py diff --git a/scripts/rewrite_errors_llm.py b/scripts/rewrite_errors_llm.py new file mode 100644 index 000000000000..3d2c9ac826f9 --- /dev/null +++ b/scripts/rewrite_errors_llm.py @@ -0,0 +1,127 @@ +# /// script +# requires-python = ">=3.11" +# dependencies = [ +# "aiohttp", +# "dotenv", +# "iofiles", +# "json", +# "openai", +# ] +# /// + + +import asyncio +import getpass +import json +import os +from pathlib import Path + +import aiofiles +import aiohttp +from dotenv import load_dotenv +from openai import AsyncOpenAI + + +def get_openai_api_key() -> str: + # Try to get the API key from the environment variable + if api_key := os.getenv("OPENAI_API_KEY"): + return api_key + + # Load environment variables from a .env file if not already loaded + load_dotenv() + if api_key := os.getenv("OPENAI_API_KEY"): + return api_key + + # Prompt the user for the API key as a last resort + return getpass.getpass("Enter your OPENAI_API_KEY: ") + + +# --- Config --- +GUIDELINES_URL = "https://raw.githubusercontent.com/ITISFoundation/osparc-simcore/refs/heads/master/docs/messages-guidelines.md" +INPUT_FILE = "errors.txt" # Supports either .txt (one per line) or .json (list) +MODEL = "gpt-4" +API_KEY = get_openai_api_key() + +client = AsyncOpenAI(api_key=API_KEY) + +# --- Functions --- + + +async def fetch_guidelines() -> str: + async with aiohttp.ClientSession() as session, session.get(GUIDELINES_URL) as resp: + return await resp.text() + + +async def load_messages(filepath: str) -> list[str]: + path = Path(filepath) + async with aiofiles.open(path) as f: + content = await f.read() + try: + return json.loads(content) + except json.JSONDecodeError: + return [line.strip() for line in content.splitlines() if line.strip()] + + +def build_system_prompt(guidelines: str) -> str: + return f""" +You are a technical writing assistant specialized in crafting professional error and warning messages. +Your task is to rewrite the given error message to strictly adhere to the oSparc Simcore message guidelines. + +Here are the guidelines: +{guidelines} + +Instructions: +- Follow all the principles from the message guidelines. +- Ensure the message is concise, user-focused, actionable, and avoids developer jargon. +- If there is not enough context, ask a clarifying question. + +Use this format only: +If enough context: +REWRITTEN: +If not enough context: +NEED MORE INFO: +""".strip() + + +async def rewrite_message(message: str, system_prompt: str, index: int) -> dict: + try: + response = await client.chat.completions.create( + model=MODEL, + messages=[ + {"role": "system", "content": system_prompt}, + {"role": "user", "content": f"ERROR: {message}"}, + ], + temperature=0, + ) + return { + "index": index, + "original": message, + "output": response.choices[0].message.content.strip(), + } + except Exception as e: + return {"index": index, "original": message, "error": str(e)} + + +# --- Main Flow --- + + +async def main(): + guidelines = await fetch_guidelines() + system_prompt = build_system_prompt(guidelines) + messages = await load_messages(INPUT_FILE) + + tasks = [ + rewrite_message(msg, system_prompt, i) for i, msg in enumerate(messages, 1) + ] + results = await asyncio.gather(*tasks) + + for result in results: + print(f"\n[{result['index']}] Original: {result['original']}") + if "output" in result: + print(result["output"]) + else: + print(f"ERROR: {result['error']}") + + +if __name__ == "__main__": + asyncio.run(main()) From 624b1281621892cff4e5955b23e75e352b795dcc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 15:09:57 +0200 Subject: [PATCH 06/21] adds instructions --- .github/copilot-instructions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7223b71aee63..3ce92882a149 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -51,5 +51,6 @@ This document provides guidelines and best practices for using GitHub Copilot in - [Python Coding Conventions](../docs/coding-conventions.md) - [Environment Variables Guide](../docs/env-vars.md) -- [Steps to Upgrade Python](../docs/steps-to-upgrade-python.md) - [Pydantic Annotated fields](../docs/llm-prompts/pydantic-annotated-fields.md) +- [Human readable Error and Warning messages address to final users](../docs/messages-guidelines.md) +- [Steps to Upgrade Python](../docs/steps-to-upgrade-python.md) From 299511bc7e20d956ba86c5e62f328c3a8cea6560 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 15:21:45 +0200 Subject: [PATCH 07/21] refactor: Remove message extraction, refinement, and reintegration scripts --- .../message_extraction.py | 59 ------------ .../message_refinement.py | 90 ------------------- .../message_reintegration.py | 43 --------- 3 files changed, 192 deletions(-) delete mode 100644 scripts/user-message-reviewer/message_extraction.py delete mode 100644 scripts/user-message-reviewer/message_refinement.py delete mode 100644 scripts/user-message-reviewer/message_reintegration.py diff --git a/scripts/user-message-reviewer/message_extraction.py b/scripts/user-message-reviewer/message_extraction.py deleted file mode 100644 index 26e7aa261292..000000000000 --- a/scripts/user-message-reviewer/message_extraction.py +++ /dev/null @@ -1,59 +0,0 @@ -import ast -import json -from pathlib import Path - -""" -def mark_for_review(message): - return message - -# Example error messages -error_message = mark_for_review("Invalid input: '{input_value}'") -not_found_message = mark_for_review("File '{filename}' was not found") -""" - - -def extract_unreviewed_messages(code_file, output_file, condition=None): - """ - Extracts messages from mark_for_review calls that match a condition on the version tag. - - Parameters: - code_file (str): Path to the Python source code file. - output_file (str): Path to save the extracted messages as a JSON file. - condition (callable, optional): A function to evaluate the version tag. If None, extracts messages with no version tag. - """ - with Path.open(code_file) as f: - tree = ast.parse(f.read()) - - messages = {} - - # Walk through the AST to find calls to `mark_for_review` - for node in ast.walk(tree): - if ( - isinstance(node, ast.Call) - and getattr(node.func, "id", None) == "mark_for_review" - ): - if len(node.args) >= 1 and isinstance(node.args[0], ast.Constant): - original_message = node.args[0].value - version = None - - # Check if a second argument (version) exists - if len(node.args) > 1 and isinstance(node.args[1], ast.Constant): - version = node.args[1].value - - # Apply the filter condition - if condition is None or condition(version): - key = f"{node.lineno}_{node.col_offset}" # Unique key based on location - messages[key] = {"message": original_message, "version": version} - - # Save extracted messages to a JSON file - with Path.open(output_file, "w") as f: - json.dump(messages, f, indent=4) - - -# Example usage -# Condition: Extract messages with no version or explicitly unreviewed (e.g., version is None or "unreviewed") -def is_unreviewed(version): - return version is None or version == "unreviewed" - - -# extract_unreviewed_messages("your_script.py", "unreviewed_messages.json", condition=is_unreviewed) diff --git a/scripts/user-message-reviewer/message_refinement.py b/scripts/user-message-reviewer/message_refinement.py deleted file mode 100644 index cbfd58fd386c..000000000000 --- a/scripts/user-message-reviewer/message_refinement.py +++ /dev/null @@ -1,90 +0,0 @@ -import json -import os -from pathlib import Path - -import openai - -# Set your OpenAI API key -openai.api_key = os.environ["OPENAPI_API_KEY"] - - -def load_rules(rules_file): - """Load rules for message refinement from a JSON file.""" - with Path.open(rules_file) as f: - return json.load(f) - - -def load_messages(messages_file): - """Load messages from the messages.json file.""" - with Path.open(messages_file) as f: - return json.load(f) - - -def send_to_chatgpt(messages, rules): - """Send messages and rules to ChatGPT and request alternatives.""" - refined_messages = {} - for key, details in messages.items(): - message = details["message"] - version = details.get("version", "unknown") - - # Prepare the prompt - prompt = f""" -You are a language expert. Apply the following rules to suggest alternatives for the given message: - -Rules: -- Tone: {rules['tone']} -- Style: {rules['style']} -- Length: {rules['length']} -- Placeholders: {rules['placeholders']} - -Original Message: "{message}" - -Provide three alternatives based on the above rules. Ensure placeholders remain intact. -""" - - # Send the request to ChatGPT - response = openai.Completion.create( - engine="text-davinci-003", prompt=prompt, max_tokens=150, temperature=0.7 - ) - - # Parse the response - alternatives = response.choices[0].text.strip() - refined_messages[key] = { - "original": message, - "version": version, - "alternatives": alternatives.split( - "\n" - ), # Split into list if alternatives are on separate lines - } - - return refined_messages - - -def save_refined_messages(output_file, refined_messages): - """Save the refined messages to a JSON file.""" - with Path.open(output_file, "w") as f: - json.dump(refined_messages, f, indent=4) - - -# Example usage -# update_messages_in_code("your_script.py", "messages.json") - - -# Main script -if __name__ == "__main__": - # File paths - messages_file = "messages.json" - rules_file = "rules.json" - output_file = "refined_messages.json" - - # Load inputs - rules = load_rules(rules_file) - messages = load_messages(messages_file) - - # Process messages with ChatGPT - refined_messages = send_to_chatgpt(messages, rules) - - # Save the refined messages - save_refined_messages(output_file, refined_messages) - - print(f"Refined messages saved to {output_file}.") diff --git a/scripts/user-message-reviewer/message_reintegration.py b/scripts/user-message-reviewer/message_reintegration.py deleted file mode 100644 index cea9aed8624d..000000000000 --- a/scripts/user-message-reviewer/message_reintegration.py +++ /dev/null @@ -1,43 +0,0 @@ -import ast -import json -from pathlib import Path - -import astor - - -def update_messages_in_code(code_file, messages_file): - # Load corrected messages from the JSON file - with Path.open(messages_file) as f: - corrected_messages = json.load(f) - - # Parse the Python code - with Path.open(code_file) as f: - tree = ast.parse(f.read()) - - # Walk through the AST to find calls to `mark_for_review` - for node in ast.walk(tree): - if ( - isinstance(node, ast.Call) - and getattr(node.func, "id", None) == "mark_for_review" - ): - # Get the original message - if isinstance(node.args[0], ast.Constant): - original_message = node.args[0].value - - # Match it with corrected messages - for key, corrected_message in corrected_messages.items(): - if original_message == corrected_message["original"]: - # Replace the message with the corrected version - node.args[0].value = corrected_message["current"] - - # Add the version as the second argument - version_node = ast.Constant(value=corrected_message["version"]) - if len(node.args) == 1: - node.args.append(version_node) - else: - node.args[1] = version_node - - # Write the updated code back to the file - updated_code = astor.to_source(tree) - with Path.open(code_file, "w") as f: - f.write(updated_code) From aa6571d79e93e4f8d6a3be22ad1d9f74baa2feb2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 15:36:48 +0200 Subject: [PATCH 08/21] adds mark --- ...messages-guidelines.md => user-messages-guidelines.md} | 4 ++-- .../common-library/src/common_library/user_messages.py | 6 ++++++ packages/common-library/tests/test_user_messages.py | 6 ++++++ .../web/server/src/simcore_service_webserver/constants.py | 4 ++-- .../products/_controller/rest_exceptions.py | 8 ++++++-- 5 files changed, 22 insertions(+), 6 deletions(-) rename docs/{messages-guidelines.md => user-messages-guidelines.md} (96%) create mode 100644 packages/common-library/src/common_library/user_messages.py create mode 100644 packages/common-library/tests/test_user_messages.py diff --git a/docs/messages-guidelines.md b/docs/user-messages-guidelines.md similarity index 96% rename from docs/messages-guidelines.md rename to docs/user-messages-guidelines.md index cc07d2c2d1fd..dbd87e8cd607 100644 --- a/docs/messages-guidelines.md +++ b/docs/user-messages-guidelines.md @@ -1,6 +1,6 @@ -# Error and Warning Message Guidelines +# User Message Guidelines -These guidelines ensure that messages are user-friendly, clear, and helpful while maintaining a professional tone. 🚀 +These guidelines ensure that error and warnings user-facing messages are user-friendly, clear, and helpful while maintaining a professional tone. 🚀 Some details: diff --git a/packages/common-library/src/common_library/user_messages.py b/packages/common-library/src/common_library/user_messages.py new file mode 100644 index 000000000000..c68326950239 --- /dev/null +++ b/packages/common-library/src/common_library/user_messages.py @@ -0,0 +1,6 @@ +def user_message(msg: str) -> str: + """Marks a message as user-facing + + NOTE: `msg` should follow docs/user-messages-guidelines.md + """ + return msg diff --git a/packages/common-library/tests/test_user_messages.py b/packages/common-library/tests/test_user_messages.py new file mode 100644 index 000000000000..e5629700f423 --- /dev/null +++ b/packages/common-library/tests/test_user_messages.py @@ -0,0 +1,6 @@ +from common_library.user_messages import user_message + + +def test_user_message() -> None: + + assert user_message("This is a user message") == "This is a user message" diff --git a/services/web/server/src/simcore_service_webserver/constants.py b/services/web/server/src/simcore_service_webserver/constants.py index 6c0dae060da9..ef963529d53a 100644 --- a/services/web/server/src/simcore_service_webserver/constants.py +++ b/services/web/server/src/simcore_service_webserver/constants.py @@ -2,6 +2,7 @@ from typing import Final +from common_library.user_messages import user_message from servicelib.aiohttp.application_keys import ( APP_AIOPG_ENGINE_KEY, APP_CONFIG_KEY, @@ -42,12 +43,11 @@ "Under development. Use WEBSERVER_DEV_FEATURES_ENABLED=1 to enable current implementation" ) - # Request storage keys RQ_PRODUCT_KEY: Final[str] = f"{__name__}.RQ_PRODUCT_KEY" -MSG_TRY_AGAIN_OR_SUPPORT: Final[str] = ( +MSG_TRY_AGAIN_OR_SUPPORT: Final[str] = user_message( "Please try again shortly. If the issue persists, contact support." ) diff --git a/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py b/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py index a9e8cb13f00c..9402486e27ea 100644 --- a/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/products/_controller/rest_exceptions.py @@ -1,3 +1,4 @@ +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...constants import MSG_TRY_AGAIN_OR_SUPPORT @@ -12,11 +13,14 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { ProductNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "{product_name} was not found", + user_message("{product_name} was not found"), ), MissingStripeConfigError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "{product_name} service is currently unavailable." + MSG_TRY_AGAIN_OR_SUPPORT, + user_message( + "{product_name} service is currently unavailable." + + MSG_TRY_AGAIN_OR_SUPPORT + ), ), } From 5dc7ad72953cccfd83ffc7060ee193b0cf7d9de9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 15:52:44 +0200 Subject: [PATCH 09/21] feat: Update error messages to use user_message for consistency across exception handlers --- .../api_keys/_controller/rest_exceptions.py | 5 ++-- .../catalog/_controller_rest_exceptions.py | 12 +++++---- .../_controller/_rest_exceptions.py | 5 ++-- .../folders/_common/exceptions_handlers.py | 27 ++++++++++++------- .../_controller/_functions_rest_exceptions.py | 4 ++- .../groups/_common/exceptions_handlers.py | 15 ++++++----- .../licenses/_common/exceptions_handlers.py | 11 +++++--- .../projects/_controller/_rest_exceptions.py | 11 ++++---- .../projects/_controller/trash_rest.py | 9 +++++-- .../simcore_service_webserver/tags/_rest.py | 15 +++++++---- .../tasks/_exception_handlers.py | 21 +++++++++------ .../simcore_service_webserver/trash/_rest.py | 9 +++++-- .../users/_users_rest.py | 23 +++++++++++----- .../workspaces/_common/exceptions_handlers.py | 15 +++++++---- 14 files changed, 117 insertions(+), 65 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py index f5888adc99ba..d18409a2df52 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py @@ -1,3 +1,4 @@ +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...exception_handling import ( @@ -11,11 +12,11 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { ApiKeyDuplicatedDisplayNameError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "API key display name duplicated", + user_message("API key display name duplicated"), ), ApiKeyNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "API key was not found", + user_message("API key was not found"), ), } diff --git a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py index 98603af76c10..4b290bf48e93 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py @@ -4,6 +4,7 @@ from aiohttp import web from common_library.error_codes import create_error_code +from common_library.user_messages import user_message from models_library.rest_error import ErrorGet from servicelib.aiohttp import status from servicelib.logging_errors import create_troubleshotting_log_kwargs @@ -85,22 +86,23 @@ async def _handler_catalog_client_errors( _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { RemoteMethodNotRegisteredError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - MSG_CATALOG_SERVICE_UNAVAILABLE, + user_message("Catalog service method unavailable. Please try again shortly."), ), CatalogForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Forbidden catalog access", + user_message("Access to catalog denied."), ), CatalogItemNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Catalog item not found", + user_message("Catalog item not found."), ), DefaultPricingPlanNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Default pricing plan not found", + user_message("Default pricing plan not found."), ), DefaultPricingUnitForServiceNotFoundError: HttpErrorInfo( - status.HTTP_404_NOT_FOUND, "Default pricing unit not found" + status.HTTP_404_NOT_FOUND, + user_message("Default pricing unit for service not found."), ), } diff --git a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py index b5bb7a6cd7ff..02a5a1fef748 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py @@ -2,6 +2,7 @@ from aiohttp import web from common_library.error_codes import create_error_code +from common_library.user_messages import user_message from models_library.rest_error import ErrorGet from servicelib import status_codes_utils from servicelib.aiohttp import status @@ -85,11 +86,11 @@ async def _handler_director_service_error_as_503_or_4xx( _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { UserDefaultWalletNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Default wallet not found but necessary for computations", + user_message("Default wallet not found but necessary for computations"), ), WalletNotEnoughCreditsError: HttpErrorInfo( status.HTTP_402_PAYMENT_REQUIRED, - "Wallet does not have enough credits for computations. {reason}", + user_message("Wallet does not have enough credits for computations. {reason}"), ), } diff --git a/services/web/server/src/simcore_service_webserver/folders/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_common/exceptions_handlers.py index d117b870c970..875f2790f5d6 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_common/exceptions_handlers.py @@ -1,5 +1,6 @@ import logging +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...exception_handling import ( @@ -31,44 +32,50 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { FolderNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Folder was not found", + user_message("Folder was not found"), ), WorkspaceNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Workspace was not found", + user_message("Workspace was not found"), ), FolderAccessForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Does not have access to this folder", + user_message("Does not have access to this folder"), ), WorkspaceAccessForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Does not have access to this workspace", + user_message("Does not have access to this workspace"), ), WorkspaceFolderInconsistencyError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "This folder does not exist in this workspace", + user_message("This folder does not exist in this workspace"), ), FolderValueNotPermittedError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Provided folder value is not permitted: {reason}", + user_message("Provided folder value is not permitted: {reason}"), ), FoldersValueError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Invalid folder value set: {reason}", + user_message("Invalid folder value set: {reason}"), ), ProjectInvalidRightsError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Access Denied: You do not have permission to move the project with UUID: {project_uuid}. Tip: Copy and paste the UUID into the search bar to locate the project.", + user_message( + "Access Denied: You do not have permission to move the project with UUID: {project_uuid}. Tip: Copy and paste the UUID into the search bar to locate the project." + ), ), # Trashing ProjectRunningConflictError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "One or more studies in this folder are in use and cannot be trashed. Please stop all services first and try again", + user_message( + "One or more studies in this folder are in use and cannot be trashed. Please stop all services first and try again" + ), ), ProjectStoppingError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "Something went wrong while stopping running services in studies within this folder before trashing. Aborting trash.", + user_message( + "Something went wrong while stopping running services in studies within this folder before trashing. Aborting trash." + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py index 4277cb874b5d..5c84f13c0f6f 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py @@ -1,6 +1,8 @@ import inspect import sys +from common_library.user_messages import user_message + from ...exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, @@ -20,7 +22,7 @@ # Dynamically create error mappings for all function-related errors cls: HttpErrorInfo( status_code=cls.status_code, - msg_template=cls.msg_template, + msg_template=user_message(cls.msg_template), ) for cls in function_error_classes if hasattr(cls, "status_code") and hasattr(cls, "msg_template") diff --git a/services/web/server/src/simcore_service_webserver/groups/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_common/exceptions_handlers.py index f0b9242fb70f..c6ffa9e7092c 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_common/exceptions_handlers.py @@ -1,5 +1,6 @@ import logging +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...exception_handling import ( @@ -23,32 +24,32 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { UserNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "User {uid} or {email} not found", + user_message("User {uid} or {email} not found"), ), GroupNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Group {gid} not found", + user_message("Group {gid} not found"), ), UserInGroupNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "User not found in group {gid}", + user_message("User not found in group {gid}"), ), UserAlreadyInGroupError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "User is already in group {gid}", + user_message("User is already in group {gid}"), ), UserInsufficientRightsError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Insufficient rights for {permission} access to group {gid}", + user_message("Insufficient rights for {permission} access to group {gid}"), ), # scicrunch InvalidRRIDError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Invalid RRID {rrid}", + user_message("Invalid RRID {rrid}"), ), ScicrunchError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Cannot get RRID since scicrunch.org service is not reachable.", + user_message("Cannot get RRID since scicrunch.org service is not reachable."), ), } diff --git a/services/web/server/src/simcore_service_webserver/licenses/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/licenses/_common/exceptions_handlers.py index e5586a5e76a9..0fc0c82f5a50 100644 --- a/services/web/server/src/simcore_service_webserver/licenses/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/licenses/_common/exceptions_handlers.py @@ -1,5 +1,6 @@ import logging +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...exception_handling import ( @@ -17,19 +18,21 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { LicensedItemNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Market item {licensed_item_id} not found.", + user_message("Market item {licensed_item_id} not found."), ), WalletAccessForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Credit account {wallet_id} forbidden.", + user_message("Credit account {wallet_id} forbidden."), ), WalletNotEnoughCreditsError: HttpErrorInfo( status.HTTP_402_PAYMENT_REQUIRED, - "Not enough credits in the credit account.", + user_message("Not enough credits in the credit account."), ), LicensedItemPricingPlanMatchError: HttpErrorInfo( status.HTTP_400_BAD_REQUEST, - "The provided pricing plan does not match the one associated with the licensed item.", + user_message( + "The provided pricing plan does not match the one associated with the licensed item." + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py index 4185089f2dc1..ee6ffe52408e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/_rest_exceptions.py @@ -2,6 +2,7 @@ import logging from collections import Counter +from common_library.user_messages import user_message from servicelib.aiohttp import status from servicelib.rabbitmq.rpc_interfaces.catalog.errors import ( CatalogForbiddenError, @@ -54,11 +55,11 @@ _FOLDER_ERRORS: ExceptionToHttpErrorMap = { FolderAccessForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Access to folder forbidden", + user_message("Access to folder forbidden"), ), FolderNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Folder not found: {reason}", + user_message("Folder not found: {reason}"), ), } @@ -66,15 +67,15 @@ _NODE_ERRORS: ExceptionToHttpErrorMap = { NodeNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Node '{node_uuid}' not found in project '{project_uuid}'", + user_message("Node '{node_uuid}' not found in project '{project_uuid}'"), ), ParentNodeNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Parent node '{node_uuid}' not found", + user_message("Parent node '{node_uuid}' not found"), ), ProjectNodeRequiredInputsNotSetError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Project node is required but input is not set", + user_message("Project node is required but input is not set"), ), } diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/trash_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/trash_rest.py index f1cae188a7de..f7e488e92d35 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/trash_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/trash_rest.py @@ -1,6 +1,7 @@ import logging from aiohttp import web +from common_library.user_messages import user_message from servicelib.aiohttp import status from servicelib.aiohttp.requests_validation import ( parse_request_path_parameters_as, @@ -28,11 +29,15 @@ _TRASH_ERRORS: ExceptionToHttpErrorMap = { ProjectRunningConflictError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Current study is in use and cannot be trashed [project_id={project_uuid}]. Please stop all services first and try again", + user_message( + "Current study is in use and cannot be trashed [project_id={project_uuid}]. Please stop all services first and try again" + ), ), ProjectStoppingError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "Something went wrong while stopping services before trashing. Aborting trash.", + user_message( + "Something went wrong while stopping services before trashing. Aborting trash." + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/tags/_rest.py b/services/web/server/src/simcore_service_webserver/tags/_rest.py index ea39edd6c2ad..10d4c86a422c 100644 --- a/services/web/server/src/simcore_service_webserver/tags/_rest.py +++ b/services/web/server/src/simcore_service_webserver/tags/_rest.py @@ -1,4 +1,5 @@ from aiohttp import web +from common_library.user_messages import user_message from servicelib.aiohttp import status from servicelib.aiohttp.requests_validation import ( parse_request_body_as, @@ -38,23 +39,27 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { TagNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Tag {tag_id} not found: either no access or does not exists", + user_message("Tag {tag_id} not found: either no access or does not exists"), ), TagOperationNotAllowedError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Could not {operation} tag {tag_id}. Not found or insuficient access.", + user_message( + "Could not {operation} tag {tag_id}. Not found or insuficient access." + ), ), ShareTagWithEveryoneNotAllowedError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Sharing with everyone is not permitted.", + user_message("Sharing with everyone is not permitted."), ), ShareTagWithProductGroupNotAllowedError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Sharing with all users is only permitted to admin users (e.g. testers, POs, ...).", + user_message( + "Sharing with all users is only permitted to admin users (e.g. testers, POs, ...)." + ), ), InsufficientTagShareAccessError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Insufficient access rightst to share (or unshare) tag {tag_id}.", + user_message("Insufficient access rightst to share (or unshare) tag {tag_id}."), ), } diff --git a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py index b0b9d8754c51..65cc64b94351 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py @@ -1,3 +1,4 @@ +from common_library.user_messages import user_message from models_library.api_schemas_rpc_async_jobs.exceptions import ( JobAbortedError, JobError, @@ -22,35 +23,39 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { InvalidFileIdentifierError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Could not find file {file_id}", + user_message("Could not find file {file_id}"), ), AccessRightError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Accessright error: user {user_id} does not have access to file {file_id} with location {location_id}", + user_message( + "Accessright error: user {user_id} does not have access to file {file_id} with location {location_id}" + ), ), JobAbortedError: HttpErrorInfo( status.HTTP_410_GONE, - "Task {job_id} is aborted", + user_message("Task {job_id} is aborted"), ), JobError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - "Task '{job_id}' failed with exception type '{exc_type}' and message: {exc_msg}", + user_message( + "Task '{job_id}' failed with exception type '{exc_type}' and message: {exc_msg}" + ), ), JobNotDoneError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "task {job_id} is not done yet", + user_message("task {job_id} is not done yet"), ), JobMissingError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "No task with id: {job_id}", + user_message("No task with id: {job_id}"), ), JobSchedulerError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - "Encountered an error with the task scheduling system", + user_message("Encountered an error with the task scheduling system"), ), JobStatusError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - "Encountered an error while getting the status of task {job_id}", + user_message("Encountered an error while getting the status of task {job_id}"), ), } diff --git a/services/web/server/src/simcore_service_webserver/trash/_rest.py b/services/web/server/src/simcore_service_webserver/trash/_rest.py index d69719840869..728aca09b647 100644 --- a/services/web/server/src/simcore_service_webserver/trash/_rest.py +++ b/services/web/server/src/simcore_service_webserver/trash/_rest.py @@ -2,6 +2,7 @@ import logging from aiohttp import web +from common_library.user_messages import user_message from servicelib.aiohttp import status from servicelib.aiohttp.application_keys import APP_FIRE_AND_FORGET_TASKS_KEY from servicelib.utils import fire_and_forget_task @@ -25,11 +26,15 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { ProjectRunningConflictError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Current study is in use and cannot be trashed [project_id={project_uuid}]. Please stop all services first and try again", + user_message( + "Current study is in use and cannot be trashed [project_id={project_uuid}]. Please stop all services first and try again" + ), ), ProjectStoppingError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "Something went wrong while stopping services before trashing. Aborting trash.", + user_message( + "Something went wrong while stopping services before trashing. Aborting trash." + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/users/_users_rest.py b/services/web/server/src/simcore_service_webserver/users/_users_rest.py index 66bfa26e3fda..7590c7fb17f6 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_rest.py @@ -3,6 +3,7 @@ from typing import Any from aiohttp import web +from common_library.user_messages import user_message from common_library.users_enums import AccountRequestStatus from models_library.api_schemas_webserver.users import ( MyProfileGet, @@ -54,25 +55,33 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { PendingPreRegistrationNotFoundError: HttpErrorInfo( status.HTTP_400_BAD_REQUEST, - PendingPreRegistrationNotFoundError.msg_template, + user_message(PendingPreRegistrationNotFoundError.msg_template), ), UserNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "This user cannot be found. Either it is not registered or has enabled privacy settings.", + user_message( + "This user cannot be found. Either it is not registered or has enabled privacy settings." + ), ), UserNameDuplicateError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Username '{user_name}' is already taken. " - "Consider '{alternative_user_name}' instead.", + user_message( + "Username '{user_name}' is already taken. " + "Consider '{alternative_user_name}' instead." + ), ), AlreadyPreRegisteredError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "Found {num_found} matches for '{email}'. Cannot pre-register existing user", + user_message( + "Found {num_found} matches for '{email}'. Cannot pre-register existing user" + ), ), MissingGroupExtraPropertiesForProductError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "The product is not ready for use until the configuration is fully completed. " - "Please wait and try again. ", + user_message( + "The product is not ready for use until the configuration is fully completed. " + "Please wait and try again. " + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_common/exceptions_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_common/exceptions_handlers.py index 32bb81224a79..4a4b77a5cca7 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_common/exceptions_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_common/exceptions_handlers.py @@ -1,5 +1,6 @@ import logging +from common_library.user_messages import user_message from servicelib.aiohttp import status from ...exception_handling import ( @@ -21,24 +22,28 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { WorkspaceGroupNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Workspace {workspace_id} group {group_id} not found.", + user_message("Workspace {workspace_id} group {group_id} not found."), ), WorkspaceAccessForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - "Does not have access to this workspace", + user_message("Does not have access to this workspace"), ), WorkspaceNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - "Workspace not found. {reason}", + user_message("Workspace not found. {reason}"), ), # Trashing ProjectRunningConflictError: HttpErrorInfo( status.HTTP_409_CONFLICT, - "One or more studies in this workspace are in use and cannot be trashed. Please stop all services first and try again", + user_message( + "One or more studies in this workspace are in use and cannot be trashed. Please stop all services first and try again" + ), ), ProjectStoppingError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - "Something went wrong while stopping running services in studies within this workspace before trashing. Aborting trash.", + user_message( + "Something went wrong while stopping running services in studies within this workspace before trashing. Aborting trash." + ), ), } From 513ed11fda71007854c30dee17f7090936d67c4a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 15:55:12 +0200 Subject: [PATCH 10/21] other user messages --- .../service-library/src/servicelib/aiohttp/rest_middlewares.py | 3 ++- .../director_v2/_controller/_rest_exceptions.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py index c8c731a2b8c3..b874a57be0c7 100644 --- a/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py +++ b/packages/service-library/src/servicelib/aiohttp/rest_middlewares.py @@ -13,6 +13,7 @@ from aiohttp.web_response import StreamResponse from common_library.error_codes import create_error_code from common_library.json_serialization import json_dumps, json_loads +from common_library.user_messages import user_message from models_library.rest_error import ErrorGet, ErrorItemType, LogMessageType from ..logging_errors import create_troubleshotting_log_kwargs @@ -31,7 +32,7 @@ from .web_exceptions_extension import get_http_error_class_or_none DEFAULT_API_VERSION = "v0" -_FMSG_INTERNAL_ERROR_USER_FRIENDLY = ( +_FMSG_INTERNAL_ERROR_USER_FRIENDLY = user_message( "We apologize for the inconvenience. " "The issue has been recorded, please report it if it persists." ) diff --git a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py index 02a5a1fef748..597b07385fb2 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py @@ -44,7 +44,7 @@ async def _handler_director_service_error_as_503_or_4xx( if status_codes_utils.is_5xx_server_error(exception.status): # NOTE: All directorv2 5XX are mapped to 503 status_code = status.HTTP_503_SERVICE_UNAVAILABLE - user_msg = ( + user_msg = user_message( # Most likely the director service is down or misconfigured so the user is asked to try again later. "This service is temporarily unavailable. The incident was logged and will be investigated. " + MSG_TRY_AGAIN_OR_SUPPORT From dee5f8aae864fa209b8e6c5bc7e7c3a2efe5776b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:14:00 +0200 Subject: [PATCH 11/21] feat: Add guidelines for updating user messages and enhance user_message function with versioning support --- .../prompts/update-user-messages.prompt.md | 92 +++++++++++++++++++ .../src/common_library/user_messages.py | 9 +- 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 .github/prompts/update-user-messages.prompt.md diff --git a/.github/prompts/update-user-messages.prompt.md b/.github/prompts/update-user-messages.prompt.md new file mode 100644 index 000000000000..46b84acd3e7c --- /dev/null +++ b/.github/prompts/update-user-messages.prompt.md @@ -0,0 +1,92 @@ +--- +mode: 'edit' +description: 'Update user messages' +--- + +This prompt guide is for updating user-facing messages in ${file} or ${selection} + +## What is a User Message? + +A user message is any string that will be displayed to end-users of the application. +In our codebase, these messages are marked with the `user_message` function: + +```python +from common_library.user_messages import user_message + +error_msg = user_message("Operation failed. Please try again later.") +``` + +## Guidelines for Updating User Messages + +When modifying user messages, follow these rules: + +1. **Version Tracking**: Every modification to a user message must include an incremented `_version` parameter: + + ```python + # Before modification + user_message("Error: Unable to connect to the server.") + + # After modification + user_message("Error: Cannot establish connection to the server.", _version=1) + ``` + +2. **F-String Preservation**: When modifying messages that use f-strings, preserve all parameters and their formatting: + + ```python + # Before + user_message(f"Project {project_name} could not be loaded.") + + # After (correct) + user_message(f"Unable to load project {project_name}.", _version=1) + + # After (incorrect - lost the parameter) + user_message("Unable to load project.", _version=1) + ``` + +3. **Message Style**: Follow the guidelines in `${workspaceFolder}/docs/user-messages-guidelines.md` + +4. **Preserve Context**: Ensure the modified message conveys the same meaning and context as the original. + +5. **Incremental Versioning**: If a message already has a version, increment it by 1: + + ```python + # Before + user_message("Session expired.", _version=2) + + # After + user_message("Your session has expired. Please log in again.", _version=3) + ``` + +## Examples + +### Example 1: Simple Message Update + +```python +# Before +error_dialog(user_message("Failed to save changes.")) + +# After +error_dialog(user_message("Failed to save your changes. Please try again.", _version=1)) +``` + +### Example 2: F-string Message Update + +```python +# Before +raise ValueError(user_message(f"Invalid input parameter: {param_name}")) + +# After +raise ValueError(user_message(f"The parameter '{param_name}' contains an invalid value.", _version=1)) +``` + +### Example 3: Already Versioned Message + +```python +# Before +return HttpErrorInfo(status.HTTP_404_NOT_FOUND, user_message("User not found.", _version=1)) + +# After +return HttpErrorInfo(status.HTTP_404_NOT_FOUND, user_message("The requested user could not be found.", _version=2)) +``` + +Remember: The goal is to improve clarity and helpfulness for end-users while maintaining accurate versioning for tracking changes. diff --git a/packages/common-library/src/common_library/user_messages.py b/packages/common-library/src/common_library/user_messages.py index c68326950239..0e92589fbb93 100644 --- a/packages/common-library/src/common_library/user_messages.py +++ b/packages/common-library/src/common_library/user_messages.py @@ -1,6 +1,11 @@ -def user_message(msg: str) -> str: +def user_message(msg: str, *, _version=None) -> str: """Marks a message as user-facing - NOTE: `msg` should follow docs/user-messages-guidelines.md + Arguments: + msg -- human-friendly string that follows docs/user-messages-guidelines.md + _version -- version number to track changes to messages; increment when modifying an existing message + + Returns: + The original message string, allowing it to be used inline in code """ return msg From 345f90fab5560a1cb3bc8a5df7cf33a23b2b28c9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:18:23 +0200 Subject: [PATCH 12/21] feat: Update user messages in exception handlers for clarity and versioning support --- .../tasks/_exception_handlers.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py index 65cc64b94351..d4ed601ba33b 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py @@ -23,39 +23,40 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { InvalidFileIdentifierError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Could not find file {file_id}"), + user_message("File {file_id} could not be found", _version=1), ), AccessRightError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, user_message( - "Accessright error: user {user_id} does not have access to file {file_id} with location {location_id}" + "Access denied: User {user_id} does not have permission to access file {file_id} in location {location_id}", + _version=1, ), ), JobAbortedError: HttpErrorInfo( status.HTTP_410_GONE, - user_message("Task {job_id} is aborted"), + user_message("Task {job_id} has been aborted", _version=1), ), JobError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, user_message( - "Task '{job_id}' failed with exception type '{exc_type}' and message: {exc_msg}" + "Task '{job_id}' failed with error type '{exc_type}': {exc_msg}", _version=1 ), ), JobNotDoneError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("task {job_id} is not done yet"), + user_message("Task {job_id} is still in progress", _version=1), ), JobMissingError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("No task with id: {job_id}"), + user_message("Task with ID {job_id} was not found", _version=1), ), JobSchedulerError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - user_message("Encountered an error with the task scheduling system"), + user_message("An error occurred in the task scheduling system", _version=1), ), JobStatusError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - user_message("Encountered an error while getting the status of task {job_id}"), + user_message("Failed to retrieve status for task {job_id}", _version=1), ), } From 5dcaf455f626f7a2c90204814064498413ce8ba1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:20:47 +0200 Subject: [PATCH 13/21] feat: Update user messages for API key errors to enhance clarity and versioning support --- .../api_keys/_controller/rest_exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py index d18409a2df52..e65fe8bfdc30 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py @@ -12,11 +12,11 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { ApiKeyDuplicatedDisplayNameError: HttpErrorInfo( status.HTTP_409_CONFLICT, - user_message("API key display name duplicated"), + user_message("An API key with this display name already exists", _version=1), ), ApiKeyNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("API key was not found"), + user_message("The requested API key could not be found", _version=1), ), } From cbd0c99ea1774beaa614fe188851a27baf3e62d2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:22:50 +0200 Subject: [PATCH 14/21] feat: Enhance user messages in exception handlers with versioning support for improved clarity --- .../catalog/_controller_rest_exceptions.py | 16 +++++++++++----- .../director_v2/_controller/_rest_exceptions.py | 5 +++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py index 4b290bf48e93..6b2a26d0db44 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py @@ -86,23 +86,29 @@ async def _handler_catalog_client_errors( _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { RemoteMethodNotRegisteredError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, - user_message("Catalog service method unavailable. Please try again shortly."), + user_message( + "Catalog service method unavailable. Please try again shortly.", _version=1 + ), ), CatalogForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, - user_message("Access to catalog denied."), + user_message( + "You do not have permission to access this catalog item.", _version=1 + ), ), CatalogItemNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Catalog item not found."), + user_message("The requested catalog item could not be found.", _version=1), ), DefaultPricingPlanNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Default pricing plan not found."), + user_message("The default pricing plan could not be found.", _version=1), ), DefaultPricingUnitForServiceNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Default pricing unit for service not found."), + user_message( + "The default pricing unit for this service could not be found.", _version=1 + ), ), } diff --git a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py index 597b07385fb2..2d866c48b387 100644 --- a/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/director_v2/_controller/_rest_exceptions.py @@ -46,8 +46,9 @@ async def _handler_director_service_error_as_503_or_4xx( status_code = status.HTTP_503_SERVICE_UNAVAILABLE user_msg = user_message( # Most likely the director service is down or misconfigured so the user is asked to try again later. - "This service is temporarily unavailable. The incident was logged and will be investigated. " - + MSG_TRY_AGAIN_OR_SUPPORT + "This service is temporarily unavailable. The incident has been logged and will be investigated. " + + MSG_TRY_AGAIN_OR_SUPPORT, + _version=1, ) # Log for further investigation From 4c15ad54f4570f0443e0638963b82f5ec69ba964 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:26:30 +0200 Subject: [PATCH 15/21] cleanup --- scripts/rewrite_errors_llm.py | 127 ------------------ .../_controller/_functions_rest_exceptions.py | 4 +- 2 files changed, 1 insertion(+), 130 deletions(-) delete mode 100644 scripts/rewrite_errors_llm.py diff --git a/scripts/rewrite_errors_llm.py b/scripts/rewrite_errors_llm.py deleted file mode 100644 index 3d2c9ac826f9..000000000000 --- a/scripts/rewrite_errors_llm.py +++ /dev/null @@ -1,127 +0,0 @@ -# /// script -# requires-python = ">=3.11" -# dependencies = [ -# "aiohttp", -# "dotenv", -# "iofiles", -# "json", -# "openai", -# ] -# /// - - -import asyncio -import getpass -import json -import os -from pathlib import Path - -import aiofiles -import aiohttp -from dotenv import load_dotenv -from openai import AsyncOpenAI - - -def get_openai_api_key() -> str: - # Try to get the API key from the environment variable - if api_key := os.getenv("OPENAI_API_KEY"): - return api_key - - # Load environment variables from a .env file if not already loaded - load_dotenv() - if api_key := os.getenv("OPENAI_API_KEY"): - return api_key - - # Prompt the user for the API key as a last resort - return getpass.getpass("Enter your OPENAI_API_KEY: ") - - -# --- Config --- -GUIDELINES_URL = "https://raw.githubusercontent.com/ITISFoundation/osparc-simcore/refs/heads/master/docs/messages-guidelines.md" -INPUT_FILE = "errors.txt" # Supports either .txt (one per line) or .json (list) -MODEL = "gpt-4" -API_KEY = get_openai_api_key() - -client = AsyncOpenAI(api_key=API_KEY) - -# --- Functions --- - - -async def fetch_guidelines() -> str: - async with aiohttp.ClientSession() as session, session.get(GUIDELINES_URL) as resp: - return await resp.text() - - -async def load_messages(filepath: str) -> list[str]: - path = Path(filepath) - async with aiofiles.open(path) as f: - content = await f.read() - try: - return json.loads(content) - except json.JSONDecodeError: - return [line.strip() for line in content.splitlines() if line.strip()] - - -def build_system_prompt(guidelines: str) -> str: - return f""" -You are a technical writing assistant specialized in crafting professional error and warning messages. -Your task is to rewrite the given error message to strictly adhere to the oSparc Simcore message guidelines. - -Here are the guidelines: -{guidelines} - -Instructions: -- Follow all the principles from the message guidelines. -- Ensure the message is concise, user-focused, actionable, and avoids developer jargon. -- If there is not enough context, ask a clarifying question. - -Use this format only: -If enough context: -REWRITTEN: -If not enough context: -NEED MORE INFO: -""".strip() - - -async def rewrite_message(message: str, system_prompt: str, index: int) -> dict: - try: - response = await client.chat.completions.create( - model=MODEL, - messages=[ - {"role": "system", "content": system_prompt}, - {"role": "user", "content": f"ERROR: {message}"}, - ], - temperature=0, - ) - return { - "index": index, - "original": message, - "output": response.choices[0].message.content.strip(), - } - except Exception as e: - return {"index": index, "original": message, "error": str(e)} - - -# --- Main Flow --- - - -async def main(): - guidelines = await fetch_guidelines() - system_prompt = build_system_prompt(guidelines) - messages = await load_messages(INPUT_FILE) - - tasks = [ - rewrite_message(msg, system_prompt, i) for i, msg in enumerate(messages, 1) - ] - results = await asyncio.gather(*tasks) - - for result in results: - print(f"\n[{result['index']}] Original: {result['original']}") - if "output" in result: - print(result["output"]) - else: - print(f"ERROR: {result['error']}") - - -if __name__ == "__main__": - asyncio.run(main()) diff --git a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py index 5c84f13c0f6f..4277cb874b5d 100644 --- a/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/functions/_controller/_functions_rest_exceptions.py @@ -1,8 +1,6 @@ import inspect import sys -from common_library.user_messages import user_message - from ...exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, @@ -22,7 +20,7 @@ # Dynamically create error mappings for all function-related errors cls: HttpErrorInfo( status_code=cls.status_code, - msg_template=user_message(cls.msg_template), + msg_template=cls.msg_template, ) for cls in function_error_classes if hasattr(cls, "status_code") and hasattr(cls, "msg_template") From afb0289b3902e76f62eb67bb6a12837f0efb7696 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:35:35 +0200 Subject: [PATCH 16/21] minor --- .github/copilot-instructions.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3ce92882a149..a1293f80e9dc 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -52,5 +52,4 @@ This document provides guidelines and best practices for using GitHub Copilot in - [Python Coding Conventions](../docs/coding-conventions.md) - [Environment Variables Guide](../docs/env-vars.md) - [Pydantic Annotated fields](../docs/llm-prompts/pydantic-annotated-fields.md) -- [Human readable Error and Warning messages address to final users](../docs/messages-guidelines.md) - [Steps to Upgrade Python](../docs/steps-to-upgrade-python.md) From 3d2fa837db85f73d055bf65cf1c031f383517fbc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:43:28 +0200 Subject: [PATCH 17/21] refactor: Remove unused error classes and factory functions from error handling module --- .../src/common_library/errors_classes.py | 44 ---------------- .../tests/test_errors_classes.py | 51 +------------------ 2 files changed, 1 insertion(+), 94 deletions(-) diff --git a/packages/common-library/src/common_library/errors_classes.py b/packages/common-library/src/common_library/errors_classes.py index 00cae26d1b25..dfee557d38ce 100644 --- a/packages/common-library/src/common_library/errors_classes.py +++ b/packages/common-library/src/common_library/errors_classes.py @@ -52,47 +52,3 @@ def error_context(self) -> dict[str, Any]: def error_code(self) -> str: assert isinstance(self, Exception), "subclass must be exception" # nosec return create_error_code(self) - - -class BaseOsparcError(OsparcErrorMixin, Exception): ... - - -class NotFoundError(BaseOsparcError): - msg_template = "{resource} not found: id='{resource_id}'" - - -class ForbiddenError(BaseOsparcError): - msg_template = "Access to {resource} is forbidden: id='{resource_id}'" - - -def make_resource_error( - resource: str, - error_cls: type[BaseOsparcError], - base_exception: type[Exception] = Exception, -) -> type[BaseOsparcError]: - """ - Factory function to create a custom error class for a specific resource. - - This function dynamically generates an error class that inherits from the provided - `error_cls` and optionally a `base_exception`. The generated error class automatically - includes the resource name and resource ID in its context and message. - - See usage examples in test_errors_classes.py - - LIMITATIONS: for the moment, exceptions produces with this factory cannot be serialized with pickle. - And therefore it cannot be used as exception of RabbitMQ-RPC interface - """ - - class _ResourceError(error_cls, base_exception): - def __init__(self, **ctx: Any): - ctx.setdefault("resource", resource) - - # guesses identifer e.g. project_id, user_id - if resource_id := ctx.get(f"{resource.lower()}_id"): - ctx.setdefault("resource_id", resource_id) - - super().__init__(**ctx) - - resource_class_name = "".join(word.capitalize() for word in resource.split("_")) - _ResourceError.__name__ = f"{resource_class_name}{error_cls.__name__}" - return _ResourceError diff --git a/packages/common-library/tests/test_errors_classes.py b/packages/common-library/tests/test_errors_classes.py index 5a6c4d5f87ad..808ed09c40dd 100644 --- a/packages/common-library/tests/test_errors_classes.py +++ b/packages/common-library/tests/test_errors_classes.py @@ -9,12 +9,7 @@ from typing import Any import pytest -from common_library.errors_classes import ( - ForbiddenError, - NotFoundError, - OsparcErrorMixin, - make_resource_error, -) +from common_library.errors_classes import OsparcErrorMixin def test_get_full_class_name(): @@ -159,47 +154,3 @@ class MyError(OsparcErrorMixin, ValueError): "message": "42 and 'missing=?'", "value": 42, } - - -def test_resource_error_factory(): - ProjectNotFoundError = make_resource_error("project", NotFoundError) - - error_1 = ProjectNotFoundError(resource_id="abc123") - assert "resource_id" in error_1.error_context() - assert error_1.resource_id in error_1.message # type: ignore - - -def test_resource_error_factory_auto_detect_resource_id(): - ProjectForbiddenError = make_resource_error("project", ForbiddenError) - error_2 = ProjectForbiddenError(project_id="abc123", other_id="foo") - assert ( - error_2.resource_id == error_2.project_id # type: ignore - ), "auto-detects project ids as resourceid" - assert error_2.other_id # type: ignore - assert error_2.code == "BaseOsparcError.ForbiddenError.ProjectForbiddenError" - - assert error_2.error_context() == { - "project_id": "abc123", - "other_id": "foo", - "resource": "project", - "resource_id": "abc123", - "message": "Access to project is forbidden: id='abc123'", - "code": "BaseOsparcError.ForbiddenError.ProjectForbiddenError", - } - - -def test_resource_error_factory_different_base_exception(): - - class MyServiceError(Exception): ... - - OtherProjectForbiddenError = make_resource_error( - "other_project", ForbiddenError, MyServiceError - ) - - assert issubclass(OtherProjectForbiddenError, MyServiceError) - - error_3 = OtherProjectForbiddenError(project_id="abc123") - assert ( - error_3.code - == "MyServiceError.BaseOsparcError.ForbiddenError.OtherProjectForbiddenError" - ) From fab6caa09767092412dba76cf9824fc86e0daf21 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 19 Jun 2025 08:01:34 +0200 Subject: [PATCH 18/21] @sanderegg review: version --- packages/common-library/src/common_library/user_messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/common-library/src/common_library/user_messages.py b/packages/common-library/src/common_library/user_messages.py index 0e92589fbb93..4c6e22cdc2e5 100644 --- a/packages/common-library/src/common_library/user_messages.py +++ b/packages/common-library/src/common_library/user_messages.py @@ -1,4 +1,4 @@ -def user_message(msg: str, *, _version=None) -> str: +def user_message(msg: str, *, _version: int | None = None) -> str: """Marks a message as user-facing Arguments: From d4dee86f3378018a221651b3af625b01fff3eacf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 19 Jun 2025 08:08:27 +0200 Subject: [PATCH 19/21] hightlighted strictly --- .../prompts/update-user-messages.prompt.md | 2 +- .../tasks/_exception_handlers.py | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/.github/prompts/update-user-messages.prompt.md b/.github/prompts/update-user-messages.prompt.md index 46b84acd3e7c..032988bcecde 100644 --- a/.github/prompts/update-user-messages.prompt.md +++ b/.github/prompts/update-user-messages.prompt.md @@ -43,7 +43,7 @@ When modifying user messages, follow these rules: user_message("Unable to load project.", _version=1) ``` -3. **Message Style**: Follow the guidelines in `${workspaceFolder}/docs/user-messages-guidelines.md` +3. **Message Style**: Follow *strictly* the guidelines in `${workspaceFolder}/docs/user-messages-guidelines.md` 4. **Preserve Context**: Ensure the modified message conveys the same meaning and context as the original. diff --git a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py index d4ed601ba33b..81b9806c57a9 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_exception_handlers.py @@ -23,40 +23,48 @@ _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { InvalidFileIdentifierError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("File {file_id} could not be found", _version=1), + user_message( + "The file with identifier {file_id} could not be found", _version=2 + ), ), AccessRightError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, user_message( - "Access denied: User {user_id} does not have permission to access file {file_id} in location {location_id}", - _version=1, + "Permission denied: You (user {user_id}) don't have the necessary rights to access file {file_id} in location {location_id}", + _version=2, ), ), JobAbortedError: HttpErrorInfo( status.HTTP_410_GONE, - user_message("Task {job_id} has been aborted", _version=1), + user_message("Task {job_id} was terminated before completion", _version=2), ), JobError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, user_message( - "Task '{job_id}' failed with error type '{exc_type}': {exc_msg}", _version=1 + "Task '{job_id}' encountered an error: {exc_msg} (error type: '{exc_type}')", + _version=2, ), ), JobNotDoneError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Task {job_id} is still in progress", _version=1), + user_message( + "Task {job_id} is still running and has not completed yet", _version=2 + ), ), JobMissingError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("Task with ID {job_id} was not found", _version=1), + user_message("The requested task with ID {job_id} does not exist", _version=2), ), JobSchedulerError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - user_message("An error occurred in the task scheduling system", _version=1), + user_message( + "The task scheduling system encountered an error. Please try again later", + _version=2, + ), ), JobStatusError: HttpErrorInfo( status.HTTP_500_INTERNAL_SERVER_ERROR, - user_message("Failed to retrieve status for task {job_id}", _version=1), + user_message("Unable to get the current status for task {job_id}", _version=2), ), } From 0a108cc5e96e0a895f5d112ea562715f7fa38641 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 19 Jun 2025 08:10:40 +0200 Subject: [PATCH 20/21] rerun nuew prompt --- .../catalog/_controller_rest_exceptions.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py index 6b2a26d0db44..244cb5a34a07 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_controller_rest_exceptions.py @@ -87,27 +87,33 @@ async def _handler_catalog_client_errors( RemoteMethodNotRegisteredError: HttpErrorInfo( status.HTTP_503_SERVICE_UNAVAILABLE, user_message( - "Catalog service method unavailable. Please try again shortly.", _version=1 + "The catalog service is temporarily unavailable. Please try again later.", + _version=2, ), ), CatalogForbiddenError: HttpErrorInfo( status.HTTP_403_FORBIDDEN, user_message( - "You do not have permission to access this catalog item.", _version=1 + "Access denied: You don't have permission to view this catalog item.", + _version=2, ), ), CatalogItemNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("The requested catalog item could not be found.", _version=1), + user_message( + "This catalog item does not exist or has been removed.", _version=2 + ), ), DefaultPricingPlanNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, - user_message("The default pricing plan could not be found.", _version=1), + user_message( + "No default pricing plan is available for this operation.", _version=2 + ), ), DefaultPricingUnitForServiceNotFoundError: HttpErrorInfo( status.HTTP_404_NOT_FOUND, user_message( - "The default pricing unit for this service could not be found.", _version=1 + "No default pricing unit is defined for this service.", _version=2 ), ), } From 7ec0c5ec0780de6219487bd5d35cb17f69b06f32 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 19 Jun 2025 08:14:41 +0200 Subject: [PATCH 21/21] tune --- .github/prompts/update-user-messages.prompt.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/prompts/update-user-messages.prompt.md b/.github/prompts/update-user-messages.prompt.md index 032988bcecde..66f9e464f3c5 100644 --- a/.github/prompts/update-user-messages.prompt.md +++ b/.github/prompts/update-user-messages.prompt.md @@ -26,8 +26,8 @@ When modifying user messages, follow these rules: # Before modification user_message("Error: Unable to connect to the server.") - # After modification - user_message("Error: Cannot establish connection to the server.", _version=1) + # After modification, add _version or increment it if it already exists + user_message("Currently unable to establish connection to the server.", _version=1) ``` 2. **F-String Preservation**: When modifying messages that use f-strings, preserve all parameters and their formatting: @@ -66,7 +66,7 @@ When modifying user messages, follow these rules: error_dialog(user_message("Failed to save changes.")) # After -error_dialog(user_message("Failed to save your changes. Please try again.", _version=1)) +error_dialog(user_message("Unable to save your changes. Please try again.", _version=1)) ``` ### Example 2: F-string Message Update @@ -76,7 +76,7 @@ error_dialog(user_message("Failed to save your changes. Please try again.", _ver raise ValueError(user_message(f"Invalid input parameter: {param_name}")) # After -raise ValueError(user_message(f"The parameter '{param_name}' contains an invalid value.", _version=1)) +raise ValueError(user_message(f"The parameter '{param_name}' contains a value that is not allowed.", _version=1)) ``` ### Example 3: Already Versioned Message