Repository: Azure/azure-functions-durable-python

Issue:
`call_http` json serializes regular `str` content.
<!-- 
Please read.

Before posting, please be sure to review whether your issue matches the description of a known regression by using [this](https://github.com/Azure/azure-functions-durable-python/issues?q=is%3Aissue+is%3Aopen+label%3Aknown-regression) query.

If your issue corresponds to a known regression, please try the suggested workarounds listed in the issue before filing an issue. Thanks!
-->
🐛 **Describe the bug**
The `call_http` method of `DurableOrchestrationContext` json serializes regular `str` content.

🤔 **Expected behavior**
It should NOT json serialize regular `str` content, as the code suggests

☕ **Steps to reproduce**
> What Durable Functions patterns are you using, if any?

N/A

> Any minimal reproducer we can use?

See: https://github.com/Azure/azure-functions-durable-python/blob/dev/azure/durable_functions/models/DurableOrchestrationContext.py#L239

The interesting block looks like:
```python
 def call_http(self, method: str, uri: str, content: Optional[str] = None,
...
        if content and content is not isinstance(content, str):
            json_content = json.dumps(content)
        else:
            json_content = content
```
and this does not seem to be quite right. If `content = "foo"` the conditions after the `if` will evaluate as follows:
```
content and (content (is not) isinstance(content, str))     # "is not" binds stronger than "and"
Truthy (and "foo" (is not) True )
True and ("foo" (is not) True)
True and True
True
```
I guess what you wanted was: 
```python
        if content and not isinstance(content, str):
            json_content = json.dumps(content)
        else:
            json_content = content
```
But if `content = []` this will result in `json_content = []` (in oposition to `json_content = "[]"`), not sure if you would want that (at least I wouldnt). You might want to use something like this, instead:
```python
        if not isinstance(content, str):
            try:
                json_content = json.dumps(content)
            except TypeError:
                json_content = content
```
Since `None` and `[]` might be desirable to put in content and are all json serializable (as `null` and `[]` resp.).
> Are you running this locally or on Azure? 

Both

⚡**If deployed to Azure**
> We have access to a lot of telemetry that can help with investigations. Please provide as much of the following information as you can to help us investigate!

N/A

> DF Version

v1.2.8



Discussion:
cgillum: It seems to me that this is a breaking change. Previously we serialized `str` as JSON, whereas with this PR, we won't serialize it anymore. To avoid breaking users that depend on the existing behavior, should we add an opt-in flag value indicating whether `str` content should be serialized?

Here's an example that introduces an optional `is_raw_str` parameter, controlling the behavior of `str` serialization.

```python
    def call_http(self, method: str, uri: str, content: Optional[str] = None,
                  headers: Optional[Dict[str, str]] = None,
                  token_source: TokenSource = None,
                  is_raw_str: bool = False) -> TaskBase:
        if content is not None:
            if isinstance(content, str) and is_raw_str:
                # don't serialize the str value - use it as the raw HTTP request payload
                json_content = content
            else:
                json_content = json.dumps(content)
```
nytian: > Hi @nytian, what are your thoughts about my suggestion to avoid a breaking change?
> 
> Even if we decide that a breaking change is the right thing to do, it would be good to add my suggested parameter (we can just change the default value) so that in the 1% chance that a customer is relying on `str` being serialized, they have a way to access the old behavior.

Thanks, @cgillum ! It's good to me as we don't want to break customer. My hesitation is around what we want to take here—whether we should leave the string as-is or serialize it into a JSON string. That's why I haven't addressed this feedback yet haha. But I will make the changes. 

Patch:
diff --git a/azure/durable_functions/models/DurableOrchestrationContext.py b/azure/durable_functions/models/DurableOrchestrationContext.py
index df10100..01ec900 100644
--- a/azure/durable_functions/models/DurableOrchestrationContext.py
+++ b/azure/durable_functions/models/DurableOrchestrationContext.py
@@ -214,7 +214,8 @@ class DurableOrchestrationContext:
 
     def call_http(self, method: str, uri: str, content: Optional[str] = None,
                   headers: Optional[Dict[str, str]] = None,
-                  token_source: TokenSource = None) -> TaskBase:
+                  token_source: TokenSource = None,
+                  is_raw_str: bool = False) -> TaskBase:
         """Schedule a durable HTTP call to the specified endpoint.
 
         Parameters
@@ -229,6 +230,9 @@ class DurableOrchestrationContext:
             The HTTP request headers.
         token_source: TokenSource
             The source of OAuth token to add to the request.
+        is_raw_str: bool, optional
+            If True, send string content as-is.
+            If False (default), serialize content to JSON.
 
         Returns
         -------
@@ -236,10 +240,21 @@ class DurableOrchestrationContext:
             The durable HTTP request to schedule.
         """
         json_content: Optional[str] = None
-        if content and content is not isinstance(content, str):
-            json_content = json.dumps(content)
-        else:
-            json_content = content
+
+        # validate parameters
+        if (not isinstance(content, str)) and is_raw_str:
+            raise TypeError(
+                "Invalid use of 'is_raw_str' parameter: 'is_raw_str' is "
+                "set to 'True' but 'content' is not an instance of type 'str'. "
+                "Either set 'is_raw_str' to 'False', or ensure your 'content' "
+                "is of type 'str'.")
+
+        if content is not None:
+            if isinstance(content, str) and is_raw_str:
+                # don't serialize the str value - use it as the raw HTTP request payload
+                json_content = content
+            else:
+                json_content = json.dumps(content)
 
         request = DurableHttpRequest(method, uri, json_content, headers, token_source)
         action = CallHttpAction(request)


Test patch:
diff --git a/tests/orchestrator/test_call_http.py b/tests/orchestrator/test_call_http.py
index be46d87..b42b36c 100644
--- a/tests/orchestrator/test_call_http.py
+++ b/tests/orchestrator/test_call_http.py
@@ -1,5 +1,6 @@
 from azure.durable_functions.models.ReplaySchema import ReplaySchema
 import json
+import pytest
 from typing import Dict
 
 from azure.durable_functions.constants import HTTP_ACTION_NAME
@@ -174,3 +175,61 @@ def test_post_completed_state():
     # assert_valid_schema(result)
     assert_orchestration_state_equals(expected, result)
     validate_result_http_request(result)
+
+@pytest.mark.parametrize("content, expected_content, is_raw_str", [
+    (None, None, False),
+    ("string data", '"string data"', False),
+    ('{"key": "value"}', '"{\\"key\\": \\"value\\"}"', False),
+    ('["list", "content"]', '"[\\"list\\", \\"content\\"]"', False),
+    ('[]', '"[]"', False),
+    ('42', '"42"', False),
+    ('true', '"true"', False),  
+    # Cases that test actual behavior (not strictly adhering to Optional[str])
+    ({"key": "value"}, '{"key": "value"}', False),
+    (["list", "content"], '["list", "content"]', False),
+    ([], '[]', False),
+    (42, '42', False),
+    (True, 'true', False),
+    # Cases when is_raw_str is True
+    ("string data", "string data", True),
+    ('{"key": "value"}', '{"key": "value"}', True),
+    ('[]', '[]', True),
+])
+def test_call_http_content_handling(content, expected_content, is_raw_str):
+    def orchestrator_function(context):
+        yield context.call_http("POST", TEST_URI, content, is_raw_str=is_raw_str)
+
+    context_builder = ContextBuilder('test_call_http_content_handling')
+    result = get_orchestration_state_result(context_builder, orchestrator_function)
+
+    assert len(result['actions']) == 1
+    http_action = result['actions'][0][0]['httpRequest']
+    
+    assert http_action['method'] == "POST"
+    assert http_action['uri'] == TEST_URI
+    assert http_action['content'] == expected_content
+
+# Test that call_http raises a TypeError when is_raw_str is True but content is not a string
+def test_call_http_non_string_content_with_raw_str():
+    def orchestrator_function(context):
+        yield context.call_http("POST", TEST_URI, {"key": "value"}, is_raw_str=True)
+
+    context_builder = ContextBuilder('test_call_http_non_string_content_with_raw_str')
+    
+    try:
+        result = get_orchestration_state_result(context_builder, orchestrator_function)
+        assert False
+    except Exception as e:
+        error_label = "\n\n$OutOfProcData$:"
+        error_str = str(e)
+
+        expected_state = base_expected_state()
+        error_msg = "Invalid use of 'is_raw_str' parameter: 'is_raw_str' is "\
+                "set to 'True' but 'content' is not an instance of type 'str'. "\
+                "Either set 'is_raw_str' to 'False', or ensure your 'content' "\
+                "is of type 'str'."
+        expected_state._error = error_msg
+        state_str = expected_state.to_json_string()
+        
+        expected_error_str = f"{error_msg}{error_label}{state_str}"
+        assert expected_error_str == error_str
\ No newline at end of file