elastic / elastic/elastic-transport-python

e.errors does not contain the most recent error

Open
#223 1 comment 1 reaction 1 assignee Claimed by @pquentin View on GitHub
Area: Client Category: Bug tracking
Dominant language
Python
Stars
24
Forks
19
PR merge metrics
No merged PRs in 30d

Description

As described here: https://elasticsearch-py.readthedocs.io/en/latest/exceptions.html

When a TlsError (or other TransportError) is initially raised, it contains a message attribute, "Cannot connect to host..." and an errors attribute which is a tuple of the causing errors, for my `TlsError` example, e.errors is a tuple with 1 item which is a `ClientConnectorCertificateError`.

When the transport invokes a request with retries (default: 3), where all retries fail (in my example due to an untrusted certificate), it takes each error encountered by each retry and appends them to a list, except for the error from the final retry which is the exception it ends up raising. [Before raising that final exception, it takes the list of previous exceptions and overwrites `e.errors` with the list of exceptions raised during previous attempts](https://github.com/elastic/elastic-transport-python/blob/47172883145b18ddd5b7f4138a58845fdc605436/elastic_transport/_async_transport.py#L326-L343).

This creates some unfortunate behavior where `e.errors` contains a list of errors which include what caused each error, but the actual most recent error (from try number 4) has now lost its `e.error`, it having been replaced with the errors from the 3 retries:

```
TLSError{ <--- Retry 3
message: "Cannot connect to host..."
errors: (
TLSError{ <--- Retry 2
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
},
TLSError{ <--- Retry 1
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
},
TLSError{ <--- Initial Request
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
}
)
}
```

Where this becomes especially problematic is when `max_retries` [is set to zero and this logic](https://github.com/elastic/elastic-transport-python/blob/47172883145b18ddd5b7f4138a58845fdc605436/elastic_transport/_async_transport.py#L334) results in `e.errors` being replaced with an empty tuple.
```
TLSError{ <-- Initial Request
message: "Cannot connect to host..."
errors: ( )
}
```

And having lost access to the underlying cause of the TLSError. It seems the only way to get access to the underlying error is to always have at least 1 retry so you've got at least 1 entry in the tuple to inspect? Though the entry will still be from the first request and not the last one so hopefully the errors are the same.

It seems perhaps that if retries is set to 0 then `e.errors` should not be overwritten, returning:
```
TLSError{
message: Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
}
```

Or perhaps e should always be appended to e.errors before raising it?
```
TLSError{ <--- Retry 3
message: "Cannot connect to host..."
errors: (
TLSError{ <--- Retry 3
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
},
TLSError{ <--- Retry 2
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
},
TLSError{ <--- Retry 1
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
},
TLSError{ <--- Initial Request
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
}
)
}
```
or in the 0 retry case:
```
TLSError{ <--- Initial Request
message: "Cannot connect to host..."
errors: (
TLSError{ <--- Initial Request
message: "Cannot connect to host..."
errors: (
ClientConnectorCertificateError,
)
}
)
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.