Repository: googleapis/proto-plus-python

Issue:
Constructing message from dictionary fails in protobuf 5.28.0 when message has a nested duration
As of protobuf 5.28.0, there is a bug in `proto-plus` where constructing a message from a dictionary fails when the message definition has a field of type `google.protobuf.duration_pb2.Duration`. Note this problem only exists when duration is in a nested message. See below:

```
    class Foo(proto.Message):
        another_field: duration_pb2.Duration = proto.Field(
            proto.MESSAGE,
            number=1,
            message=duration_pb2.Duration,
        )

    # this works
    Foo(another_field="300s")

    class Bar(proto.Message):
        some_field = proto.Field(proto.MESSAGE, number=1, message=Foo)

    # this raises `AttributeError: Fail to convert to Duration. Expected a timedelta like object got str: 'str' object has no attribute 'seconds'`
    bar = Bar({"some_field":{"another_field":"300s"}})
```

There is a change in the code path of message construction in proto-plus when using protobuf 5.28.0 and newer

In 5.27.5, we have
```
MessageMeta.__new__ → marshal.to_proto() → BaseMarshal.get_rule() → MessageRule.to_proto() → TypeError
```
where the exception is `TypeError: Message must be initialized with a dict: _default_package.Foo` raised at the location below`

In 5.28.0, we have 
```
MessageMeta.__new__ → marshal.to_proto() → BaseMarshal.get_rule() → MessageRule.to_proto() → AttributeError
```
where the exception is `AttributeError: Fail to convert to Duration. Expected a timedelta like object got str: 'str' object has no attribute 'seconds'`

Technically, this is not a breaking change in protobuf as they simply raised a different error. Proto-plus expected `TypeError` but `AttributeError` was raised instead.

See the re-production below:

In protobuf 5.27.5, run the code below
```
def test_duration_write_string_nested():
    class Foo(proto.Message):
        another_field: duration_pb2.Duration = proto.Field(
            proto.MESSAGE,
            number=1,
            message=duration_pb2.Duration,
        )

    class Bar(proto.Message):
        some_field = proto.Field(proto.MESSAGE, number=1, message=Foo)

    bar = Bar({"some_field":{"another_field":"300s"}})
    assert isinstance(bar.some_field.another_field, timedelta)
    assert isinstance(Bar.pb(bar).some_field.another_field, duration_pb2.Duration)
    assert bar.some_field.another_field.seconds == 300
    assert Bar.pb(bar).some_field.another_field.seconds== 300
```
You'll see `TypeError: Message must be initialized with a dict: _default_package.Foo` raised at the location below`

https://github.com/googleapis/proto-plus-python/blob/c54b31d71489785f20e5f140b8c43c7311fbfd97/proto/marshal/rules/message.py#L34-L37

In protobuf 5.27.5, the `TypeError` is caught, and then we successfully run the following code:
https://github.com/googleapis/proto-plus-python/blob/c54b31d71489785f20e5f140b8c43c7311fbfd97/proto/marshal/rules/message.py#L38-L45

In protobuf 5.28.0 and newer, you'll see `AttributeError` instead which is not caught in the code below and instead `AttributeError` is returned to the user

https://github.com/googleapis/proto-plus-python/blob/c54b31d71489785f20e5f140b8c43c7311fbfd97/proto/marshal/rules/message.py#L34-L45

Discussion:

Solution patch:
diff --git a/proto/marshal/rules/message.py b/proto/marshal/rules/message.py
index 479a2d9..c191180 100644
--- a/proto/marshal/rules/message.py
+++ b/proto/marshal/rules/message.py
@@ -34,14 +34,15 @@ class MessageRule:
             try:
                 # Try the fast path first.
                 return self._descriptor(**value)
-            except (TypeError, ValueError) as ex:
-                # If we have a TypeError or Valueerror,
+            except (TypeError, ValueError, AttributeError) as ex:
+                # If we have a TypeError, ValueError or AttributeError,
                 # try the slow path in case the error
                 # was:
                 # - an int64/string issue.
                 # - a missing key issue in case a key only exists with a `_` suffix.
                 #   See related issue: https://github.com/googleapis/python-api-core/issues/227.
-                # - a missing key issue due to nested struct. See: b/321905145.
+                # - a missing key issue due to nested struct. See: https://github.com/googleapis/proto-plus-python/issues/424.
+                # - a missing key issue due to nested duration. See: https://github.com/googleapis/google-cloud-python/issues/13350.
                 return self._wrapper(value)._pb
         return value
 


Test patch:
diff --git a/tests/test_marshal_types_dates.py b/tests/test_marshal_types_dates.py
index 21841b9..44c65eb 100644
--- a/tests/test_marshal_types_dates.py
+++ b/tests/test_marshal_types_dates.py
@@ -241,6 +241,26 @@ def test_duration_write_string():
     assert Foo.pb(foo).ttl.seconds == 120
 
 
+def test_duration_write_string_nested():
+    class Foo(proto.Message):
+        foo_field: duration_pb2.Duration = proto.Field(
+            proto.MESSAGE,
+            number=1,
+            message=duration_pb2.Duration,
+        )
+
+    Foo(foo_field="300s")
+
+    class Bar(proto.Message):
+        bar_field = proto.Field(proto.MESSAGE, number=1, message=Foo)
+
+    bar = Bar({"bar_field": {"foo_field": "300s"}})
+    assert isinstance(bar.bar_field.foo_field, timedelta)
+    assert isinstance(Bar.pb(bar).bar_field.foo_field, duration_pb2.Duration)
+    assert bar.bar_field.foo_field.seconds == 300
+    assert Bar.pb(bar).bar_field.foo_field.seconds == 300
+
+
 def test_duration_del():
     class Foo(proto.Message):
         ttl = proto.Field(