serhiy-bzhezytskyy commented on PR #4638: URL: https://github.com/apache/solr/pull/4638#issuecomment-5100447690
Thanks Mark. On the exception chain — you're right that the assumption is fragile, and this PR arguably adds to it: the new branch wraps a `Throwable` in a `SolrServerException` so the message can carry `basePath`, which means callers still have to know to look for that type. Unrolling to the root cause is the better shape. I'd rather keep it out of this PR, since the change here is meant to be the narrow "which node was it?" fix, and unrolling changes what every `handleError` caller *observes* — the three call sites are all in `ConcurrentUpdateBaseSolrClient`, but the behaviour is public through the overridable `handleError(Throwable)`, so it's a small diff with a visible contract change. Let me open a separate issue for it and reference your note. It would also help `UpdateErrorHandler` (SOLR-3284, and SOLR-18308 which proposes making it the preferred mechanism) report the real cause instead of the wrapper. Is that the split you'd expect, or would you rather see them together? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
