Skip to content

e.errors does not contain the most recent error #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
strawgate opened this issue Jan 10, 2025 · 1 comment · May be fixed by #224
Open

e.errors does not contain the most recent error #223

strawgate opened this issue Jan 10, 2025 · 1 comment · May be fixed by #224
Assignees
@pquentin
Labels
Area: Client Manually written code that fits in no other area Category: Bug Something isn't working

Comments

@strawgate
Copy link

strawgate commented Jan 10, 2025 •
edited
Loading

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.

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 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,
      )
    }
  )
}
@strawgate strawgate mentioned this issue Jan 10, 2025
Closed
@pquentin
Copy link
Member

Thank you for the thorough analysis! I like your last proposal best as it's more consistent with the existing behavior. I'll look into implementing it.

@pquentin pquentin added Category: Bug Something isn't working Area: Client Manually written code that fits in no other area labels Jan 14, 2025
@pquentin pquentin self-assigned this Jan 14, 2025
@pquentin pquentin linked a pull request Jan 14, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pquentin pquentin

Labels
Area: Client Manually written code that fits in no other area Category: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stop hiding cause of last exception pquentin/elastic-transport-python
2 participants
@pquentin @strawgate