diff --git a/dashscope/applications/finetune/reinforcement/common/model.py b/dashscope/applications/finetune/reinforcement/common/model.py index 31ffa4a8..b37543a4 100644 --- a/dashscope/applications/finetune/reinforcement/common/model.py +++ b/dashscope/applications/finetune/reinforcement/common/model.py @@ -107,6 +107,10 @@ _LAYER_STATUS_QUERY_SOLUTION = ( "Retry the layer status query after connectivity recovers." ) +_LAYER_STATUS_REJECTED_SOLUTION = ( + "Review the server response and layer code. For region lookup errors, " + "report the request ID and layer code to the service owner before retrying." +) _LAYER_BUILD_FAILED_SOLUTION = ( "Check that requirements.txt or setup.py is valid and every dependency " "is installable, then review the layer build logs before retrying." @@ -121,6 +125,15 @@ ) +def _fc_response_rejected(result: Dict[str, Any]) -> bool: + status = result.get("status") + return ( + (isinstance(status, dict) and status.get("code", 200) != 200) + or result.get("code") not in (None, "", 0, 200) + or result.get("success") is False + ) + + class MountStorage(BaseModel): model_config = ConfigDict(extra="forbid") @@ -447,14 +460,27 @@ async def create_layer( # "signed_url": oss_signed_url, }, ) - if result.get("status", {}).get("code", 500) != 200: + if _fc_response_rejected(result): raise FunctionLayerError( - f"create rejected by server | {result}", + f"create rejected by server | {deep_mask(result)}", + layer_name=layer_name, + request_id=result.get("request_id"), + body=result, allow_retry=False, solution=_LAYER_CREATE_REJECTED_SOLUTION, ) - layer_code = result.get("output", {}).get("layer_code", "") + layer_code = (result.get("output") or {}).get("layer_code", "") + if not layer_code: + raise FunctionLayerError( + f"Empty layer code received: {deep_mask(result)}", + error_code=2013, + layer_name=layer_name, + request_id=result.get("request_id"), + body=result, + allow_retry=False, + solution=_LAYER_CREATE_REJECTED_SOLUTION, + ) logger.debug( f"Create function layer | layer-name: {layer_name} | " f"layer_code: {layer_code}", @@ -564,7 +590,19 @@ async def get_layer( solution=_LAYER_STATUS_QUERY_SOLUTION, ) from e - status = result.get("output", {}).get("status", "") + if _fc_response_rejected(result): + raise FunctionLayerError( + f"status query rejected by server | layer_code: " + f"{layer_code} | {deep_mask(result)}", + error_code=2016, + layer_code=layer_code, + request_id=result.get("request_id"), + body=result, + allow_retry=False, + solution=_LAYER_STATUS_REJECTED_SOLUTION, + ) + + status = (result.get("output") or {}).get("status", "") if status == "SUCCESS": logger.debug( @@ -579,6 +617,8 @@ async def get_layer( f"{layer_code} | {result}", error_code=2015, layer_code=layer_code, + request_id=result.get("request_id"), + body=result, allow_retry=False, solution=_LAYER_BUILD_FAILED_SOLUTION, ) @@ -589,6 +629,8 @@ async def get_layer( f"{layer_code} | {result}", error_code=2016, layer_code=layer_code, + request_id=result.get("request_id"), + body=result, allow_retry=True, solution=_LAYER_STATUS_MISSING_SOLUTION, ) @@ -808,6 +850,8 @@ async def register( message=f"Function layer deployment failed: {root}", ), output={}, + request_id=e.request_id, + body=e.body, ) except Exception as e: root = e @@ -934,22 +978,36 @@ async def load( url, runtime_dict, ) - self.instance_id = result.get("output", {}).get("instanceId", "") + if _fc_response_rejected(result): + raise FunctionLoadError( + f"load rejected by server | {deep_mask(result)}", + error_code=2023, + entity_id=target_entity_id, + request_id=result.get("request_id"), + body=result, + allow_retry=False, + ) + + output = result.get("output") or {} + self.instance_id = output.get("instanceId", "") if not self.instance_id: raise FunctionLoadError( - f"Empty instance ID received: {result}", + f"Empty instance ID received: {deep_mask(result)}", error_code=2023, + entity_id=target_entity_id, + request_id=result.get("request_id"), + body=result, ) - self.instance_url = result.get("output", {}).get("trigger_url", "") - self.instance_token = result.get("output", {}).get( - "trigger_token", - "", - ) + self.instance_url = output.get("trigger_url", "") + self.instance_token = output.get("trigger_token", "") if (not self.instance_url) or (not self.instance_token): raise FunctionLoadError( "Missing instance URL or token", error_code=2024, + entity_id=target_entity_id, + request_id=result.get("request_id"), + body=result, ) logger.info( @@ -978,6 +1036,8 @@ async def load( ), ), output={}, + request_id=e.request_id, + body=e.body, ) except Exception as e: @@ -998,6 +1058,8 @@ async def load( ), ), output={}, + request_id=getattr(e, "request_id", None), + body=getattr(e, "body", None), ) # Perform instance warmup if requested @@ -1627,6 +1689,8 @@ async def register_functions( raise RegistrationError( reg_result.status.message, error_code=2052, + request_id=reg_result.request_id, + body=reg_result.body, ) if fc.type == FunctionType.ROLLOUT: @@ -1661,8 +1725,11 @@ async def register_functions( instance_group_reward_ids.append(instance_id) else: raise FunctionLoadError( - f"Load failed: {load_result}", + f"Load failed: {load_result.status.message}", error_code=2054, + entity_id=entity_id, + request_id=load_result.request_id, + body=load_result.body, ) except Exception as e: diff --git a/dashscope/applications/finetune/reinforcement/common/model_types.py b/dashscope/applications/finetune/reinforcement/common/model_types.py index 025b8f03..93fbaf3b 100644 --- a/dashscope/applications/finetune/reinforcement/common/model_types.py +++ b/dashscope/applications/finetune/reinforcement/common/model_types.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from enum import Enum from typing import Optional, List, Any, Dict -from pydantic import BaseModel +from pydantic import BaseModel, Field class StatusType(str, Enum): @@ -91,8 +91,17 @@ class RequestFC(BaseModel): class ResponseFC(BaseModel): + """FC result with optional upstream failure context. + + ``request_id`` and ``body`` retain the original FaaS rejection without + replacing the SDK's existing numeric ``status.code``. The raw ``body`` + may contain credentials and is excluded from the model's repr. + """ + status: Status = Status() output: Optional[Dict[str, Any]] = None + request_id: Optional[str] = None + body: Optional[Dict[str, Any]] = Field(default=None, repr=False) class RequestTuning(BaseModel): diff --git a/tests/legacy_1x/unit/test_agentic_rl_layer_status.py b/tests/legacy_1x/unit/test_agentic_rl_layer_status.py index 0e70ac18..4a91d0f7 100644 --- a/tests/legacy_1x/unit/test_agentic_rl_layer_status.py +++ b/tests/legacy_1x/unit/test_agentic_rl_layer_status.py @@ -132,8 +132,10 @@ async def test_get_layer_raises_on_region_rejection(monkeypatch): await FunctionComponentModel().get_layer(LAYER_CODE) assert "cn-wulanchabu" in str(excinfo.value) - assert excinfo.value.allow_retry is True - assert "Retry the status query" in excinfo.value.solution + assert excinfo.value.allow_retry is False + assert "service owner" in excinfo.value.solution + assert excinfo.value.request_id == LAYER_NOT_FOUND_BODY["request_id"] + assert excinfo.value.body == LAYER_NOT_FOUND_BODY assert calls["count"] == 1 @@ -235,6 +237,9 @@ async def test_load_reports_server_rejection_with_context(monkeypatch): assert ENTITY_ID in result.status.message assert LAYER_CODE in result.status.message assert "cn-wulanchabu" in result.status.message + assert "Empty instance ID" not in result.status.message + assert result.request_id == LAYER_NOT_FOUND_BODY["request_id"] + assert result.body == LAYER_NOT_FOUND_BODY async def test_load_succeeds_once_layer_is_ready(monkeypatch): diff --git a/tests/legacy_1x/unit/test_cli_main.py b/tests/legacy_1x/unit/test_cli_main.py index 203067df..c9ffdbcf 100644 --- a/tests/legacy_1x/unit/test_cli_main.py +++ b/tests/legacy_1x/unit/test_cli_main.py @@ -10,6 +10,7 @@ from types import SimpleNamespace import pytest +from typer.main import get_command from typer.testing import CliRunner import dashscope @@ -111,8 +112,11 @@ def test_rl_register_functions_hyphen_alias_help(self): ) assert result.exit_code == 0 - clean_output = strip_ansi_codes(result.output) - assert "rollout-classpaths" in clean_output + command = get_command(agentic_rl.app).commands["register-functions"] + assert any( + "--rollout-classpaths" in parameter.opts + for parameter in command.params + ) def test_rl_test_functions_hyphen_alias_help(self): result = CliRunner().invoke( diff --git a/tests/test_agentic_rl_fc_errors.py b/tests/test_agentic_rl_fc_errors.py new file mode 100644 index 00000000..30fef8ad --- /dev/null +++ b/tests/test_agentic_rl_fc_errors.py @@ -0,0 +1,264 @@ +# -*- coding: utf-8 -*- +"""Preserve FaaS business failures even when the transport reports success.""" + +from unittest.mock import AsyncMock + +import pytest + +from dashscope.applications.finetune.reinforcement import ( + FunctionComponentModel, + FunctionComponentRuntime, + FunctionLayerError, + FunctionLoadError, + RolloutFunctionComponent, + TuningModel, +) +from dashscope.applications.finetune.reinforcement.common import model + + +LAYER_CODE = "layer_9328e5982668484d9d5269a62868f6f8" +ENTITY_ID = "ro-48aa9fc3-30ec-45c7-bf4f-920f9dcb7d16" +REGION_REJECTION = { + "request_id": "577f2378-85a2-95ae-902d-444b2dac0045", + "code": "BadRequest", + "message": ( + "FaaS layer not found for region null " + "(globalLayerCode: global_layer_0f1c73df65d64214a3a0c506eff3bf4c)" + ), + "status": {"code": 200, "message": "OK"}, +} +INSTANCE_OUTPUT = { + "instanceId": "in-1", + "trigger_url": "https://example.com/invoke", + "trigger_token": "sensitive-trigger-token", +} + + +@pytest.fixture +def component(monkeypatch): + monkeypatch.setattr(model, "FC_LAYER_USED", True) + return RolloutFunctionComponent( + name="rollout-1", + entity_id=ENTITY_ID, + fcmodel=FunctionComponentModel(), + runtime=FunctionComponentRuntime(layer_code=LAYER_CODE), + ) + + +@pytest.mark.parametrize("output", [None, {}, {"layer_code": LAYER_CODE}]) +async def test_create_layer_rejects_business_failure(monkeypatch, output): + response = {**REGION_REJECTION, "output": output} + monkeypatch.setattr(model, "client_fc", AsyncMock(return_value=response)) + + with pytest.raises(FunctionLayerError) as excinfo: + await FunctionComponentModel().create_layer(requirements_file="") + + error = excinfo.value + assert "create rejected by server" in str(error) + assert "region null" in str(error) + assert error.request_id == response["request_id"] + assert error.body == response + assert error.code == "sdk.agentic_rl.FunctionLayerError" + assert error.allow_retry is False + + +@pytest.mark.parametrize("output", [None, {}, {"layer_code": ""}]) +async def test_create_layer_requires_nonempty_layer_code(monkeypatch, output): + response = {"status": {"code": 200}, "output": output} + monkeypatch.setattr(model, "client_fc", AsyncMock(return_value=response)) + + with pytest.raises(FunctionLayerError, match="Empty layer code"): + await FunctionComponentModel().create_layer(requirements_file="") + + +@pytest.mark.parametrize("output", [None, {}, {"status": "SUCCESS"}]) +async def test_get_layer_rejects_business_failure(monkeypatch, output): + response = {**REGION_REJECTION, "output": output} + client = AsyncMock(return_value=response) + monkeypatch.setattr(model, "client_fc", client) + + with pytest.raises(FunctionLayerError) as excinfo: + await FunctionComponentModel().get_layer(LAYER_CODE) + + error = excinfo.value + assert "status query rejected by server" in str(error) + assert error.request_id == response["request_id"] + assert error.body == response + assert error.layer_code == LAYER_CODE + assert error.allow_retry is False + assert client.await_count == 1 + + +@pytest.mark.parametrize("output", [None, {}, INSTANCE_OUTPUT]) +async def test_load_preserves_business_failure(monkeypatch, component, output): + response = {**REGION_REJECTION, "output": output} + client = AsyncMock( + side_effect=[{"output": {"status": "SUCCESS"}}, response], + ) + monkeypatch.setattr(model, "client_fc", client) + + result = await component.load() + + assert not result.status.success + assert result.status.name == "FunctionLoadError" + assert result.status.code == 522 + assert "load rejected by server" in result.status.message + assert "Empty instance ID" not in result.status.message + assert "region null" in result.status.message + assert ENTITY_ID in result.status.message + assert LAYER_CODE in result.status.message + assert result.request_id == response["request_id"] + assert result.body == response + assert INSTANCE_OUTPUT["trigger_token"] not in result.status.message + assert INSTANCE_OUTPUT["trigger_token"] not in repr(result) + assert component.instance_id is None + assert client.await_count == 2 + + +@pytest.mark.parametrize("output", [None, {}]) +async def test_load_missing_instance_is_still_reported( + monkeypatch, + component, + output, +): + response = {"request_id": "missing-instance", "output": output} + monkeypatch.setattr( + model, + "client_fc", + AsyncMock(side_effect=[{"output": {"status": "SUCCESS"}}, response]), + ) + + result = await component.load() + + assert not result.status.success + assert "Empty instance ID" in result.status.message + assert "NoneType" not in result.status.message + assert result.request_id == response["request_id"] + assert result.body == response + + +async def test_get_layer_missing_status_remains_retryable(monkeypatch): + response = {"request_id": "missing-status", "output": None} + monkeypatch.setattr(model, "client_fc", AsyncMock(return_value=response)) + + with pytest.raises(FunctionLayerError) as excinfo: + await FunctionComponentModel().get_layer(LAYER_CODE) + + assert "no status" in str(excinfo.value) + assert excinfo.value.allow_retry is True + assert excinfo.value.request_id == response["request_id"] + assert excinfo.value.body == response + + +async def test_registration_preserves_layer_creation_failure( + monkeypatch, + component, + tmp_path, +): + component.fcmodel.zipdir = str(tmp_path) + (tmp_path / "requirements.txt").write_text("") + monkeypatch.setattr(FunctionComponentModel, "to_oss", AsyncMock()) + client = AsyncMock(return_value=REGION_REJECTION) + monkeypatch.setattr(model, "client_fc", client) + + result = await component.register( + oss_id="oss-id", + oss_url="https://example.com/package", + ) + + assert not result.status.success + assert result.status.code == 524 + assert result.request_id == REGION_REJECTION["request_id"] + assert result.body == REGION_REJECTION + assert "create rejected by server" in result.status.message + assert client.await_count == 1 + + +async def test_load_preserves_layer_query_rejection(monkeypatch, component): + client = AsyncMock(return_value=REGION_REJECTION) + monkeypatch.setattr(model, "client_fc", client) + + result = await component.load() + + assert not result.status.success + assert result.status.name == "FunctionLayerError" + assert result.status.code == 526 + assert result.request_id == REGION_REJECTION["request_id"] + assert result.body == REGION_REJECTION + assert client.await_count == 1 + + +@pytest.mark.parametrize( + "failure", + [{"status": {"code": 400}}, {"success": False}], +) +async def test_load_rejects_other_failure_envelopes( + monkeypatch, + component, + failure, +): + client = AsyncMock( + side_effect=[ + {"output": {"status": "SUCCESS"}}, + {**failure, "output": INSTANCE_OUTPUT}, + ], + ) + monkeypatch.setattr(model, "client_fc", client) + + result = await component.load() + + assert not result.status.success + assert "load rejected by server" in result.status.message + assert component.instance_id is None + + +@pytest.mark.parametrize("code", [None, "", 0, 200]) +async def test_successful_fc_responses_remain_compatible( + monkeypatch, + component, + code, +): + monkeypatch.setattr( + model, + "client_fc", + AsyncMock( + side_effect=[ + { + "code": code, + "status": {"code": 200}, + "output": {"layer_code": LAYER_CODE}, + }, + {"code": code, "output": {"status": "SUCCESS"}}, + {"code": code, "output": INSTANCE_OUTPUT}, + ], + ), + ) + + layer_code = await component.fcmodel.create_layer(requirements_file="") + result = await component.load() + + assert layer_code == LAYER_CODE + assert result.status.success + assert result.output["instance_id"] == "in-1" + + +async def test_eager_registration_preserves_load_failure( + monkeypatch, + component, +): + client = AsyncMock( + side_effect=[{"output": {"status": "SUCCESS"}}, REGION_REJECTION], + ) + monkeypatch.setattr(model, "client_fc", client) + tuning = TuningModel(functions=[component]) + + with pytest.raises(FunctionLoadError) as excinfo: + await tuning.register_functions(lazy_load=False) + + error = excinfo.value + assert error.request_id == REGION_REJECTION["request_id"] + assert error.body == REGION_REJECTION + assert error.code == "sdk.agentic_rl.FunctionLoadError" + assert error.entity_id == ENTITY_ID + assert "region null" in str(error) + assert client.await_count == 2