Skip to content
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.

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

Provide error class for ConcurrentSnapshotExecutionException? #89

Closed
msonnabaum opened this issue Aug 27, 2014 · 4 comments
Closed

Provide error class for ConcurrentSnapshotExecutionException? #89

msonnabaum opened this issue Aug 27, 2014 · 4 comments
Labels

Comments

@msonnabaum
Copy link

I've ended up with code like this to handle a concurrent snapshot exception:

  begin
    # try to make a snapshot
  rescue Elasticsearch::Transport::Transport::Errors::ServiceUnavailable => e
    if e.message.match /ConcurrentSnapshotExecutionException/
      sleep 30
      retry
    end

    raise e
  end

This error seems semantically different enough from a typical 503 that it warrants it's own class. Thoughts?

@karmi
Copy link
Contributor

karmi commented Sep 2, 2014

@msonnabaum Yes, end users shouldn't be forced to write defensive code like this... We have discussed the topic of having concrete exceptions for various "server errors" quite frequently, across the languages (Python, PHP, JavaScript, ...) but haven't reached a conclusion... Let's keep this open, we'll discuss it again. I'm personally absolutely fine with having an exception for this, we just need to keep the exceptions organized and well accessible in docs/code.

@karmi karmi added the feature label Sep 2, 2014
@karmi
Copy link
Contributor

karmi commented Apr 27, 2015

@msonnabaum Just returned to this when sorting through the list of PRs and issues. I still like to have specific exception classes in these cases.

Notice elastic/elasticsearch#10643 which aims at changing the error responses from Elasticsearch, so they provide a structured, detailed information about the error.

I guess this is something the other APIs should/will provide as well, and we definitely want to return specific exception classes and information from these responses.

@karmi
Copy link
Contributor

karmi commented May 20, 2015

@msonnabaum I think I'll close this, since we will definitely need to add support for structured logging outlined in elastic/elasticsearch#10643 across the clients, if it's OK?

@msonnabaum
Copy link
Author

Yep, sounds reasonable to me.

@karmi karmi closed this as completed May 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants