paul-rogers commented on issue #1455: DRILL-6724: Convert IndexOutOfBounds exception to UserException with … URL: https://github.com/apache/drill/pull/1455#issuecomment-418433987 @KazydubB, thanks for the explanation. You are inserting code everywhere you believe an IOBE could be thrown. Since the exception is due to a coding error, you should, in theory, wrap all possible places. But, one could apply this same argument to NPE, division by zero and other programing errors. Soon, every line of code must be wrapped to catch programming errors. And, what can the user do? Only call support or post to the dev list. We then look at logs: the stack trace, an create a test case. So, we could adopt a more general approach: * If the error is something the user can correct (bad name, external system problem, resource problem), log a detailed error for the user. Catch the problem close to the cause so we can provide context. * If the error is a programming problem, catch it generically in the fragment executor. The original exception has the stack trace with is what the developer needs to fix the issue. Tell the user, "System error -- contact your vendor. Preserve drillbit.log for host foo.bar.com and provide it to the developer." If we do that, then your wrapper code is not needed unless it indicates an error the user can fix. Instead, improve the wording of the generic "something went wrong" error in the fragment executor. That is, there is no reason to add complexity to the code to try to handle all possible programming errors. Instead, focus that effort on better testing to find and fix such errors. Said another way, the code being added assumes errors exist, we don't know about them, and the user would like lots of details about how we screwed up in the code. That is, IMHO, an incorrect read. Uses want working code, and want to know how to correct errors due to their application or environment. Note that this is policy is not unique to each plugin. Rather, it is a project-wide guideline that might want to be discussed on the dev list.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
