Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1413

tsql should not print a stacktrace message of not critical exceptions

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Not A Problem
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: None
    • Labels:
      None

      Description

      See the following error message. It should have printed just ERROR: no such a table: table2 instead of full stacktrace message.

      default> \d table2
      ERROR: no such a table: table2
      com.google.protobuf.ServiceException: java.sql.SQLException: ERROR: no such a table: table2
      	at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:104)
      	at org.apache.tajo.client.CatalogAdminClientImpl.getTableDesc(CatalogAdminClientImpl.java:216)
      	at org.apache.tajo.client.TajoClientImpl.getTableDesc(TajoClientImpl.java:217)
      	at org.apache.tajo.cli.tsql.commands.DescTableCommand.invoke(DescTableCommand.java:50)
      	at org.apache.tajo.cli.tsql.TajoCli.executeMetaCommand(TajoCli.java:471)
      	at org.apache.tajo.cli.tsql.TajoCli.executeParsedResults(TajoCli.java:445)
      	at org.apache.tajo.cli.tsql.TajoCli.runShell(TajoCli.java:419)
      	at org.apache.tajo.cli.tsql.TajoCli.main(TajoCli.java:694)
      Caused by: java.sql.SQLException: ERROR: no such a table: table2
      	at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:231)
      	at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:217)
      	at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:94)
      	... 7 more
      
      1. TAJO-1413.Hyun.150328.1.patch.txt
        2 kB
        Dongjoon Hyun
      2. TAJO-1413.patch
        0.8 kB
        Dongjoon Hyun

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/433

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/433
        Hide
        hyunsik Hyunsik Choi added a comment -

        This issue was already fixed.

        Show
        hyunsik Hyunsik Choi added a comment - This issue was already fixed.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/433#issuecomment-89756306

        In TSQL commands, I found the following 4 trivial error patterns and suppress them. I think those cases covers almost trivial user errors.

        ```
        + if (e instanceof IllegalArgumentException)

        { + onError(e.getMessage() == null ? "ERROR: Illegal Argument" : e.getMessage(), null); + }

        else if (e instanceof IOException && invoked instanceof ExecExternalShellCommand)

        { + onError(e.getMessage(), null); + }

        else if (e.getCause() instanceof SQLException)

        { + SQLException se = (SQLException) e.getCause(); + onError(se.getMessage(), null); + }

        else

        { + onError(null, e); + }

        ```

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/433#issuecomment-89756306 In TSQL commands, I found the following 4 trivial error patterns and suppress them. I think those cases covers almost trivial user errors. ``` + if (e instanceof IllegalArgumentException) { + onError(e.getMessage() == null ? "ERROR: Illegal Argument" : e.getMessage(), null); + } else if (e instanceof IOException && invoked instanceof ExecExternalShellCommand) { + onError(e.getMessage(), null); + } else if (e.getCause() instanceof SQLException) { + SQLException se = (SQLException) e.getCause(); + onError(se.getMessage(), null); + } else { + onError(null, e); + } ```
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/433#issuecomment-89549997

        @jihoonson , Thank you for your comment. Now, I see the policy more clearly than before. I will revert the default configuration and add more case-by-case error handling code according to your opinion. Also, I will keep my eyes on this issue.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/433#issuecomment-89549997 @jihoonson , Thank you for your comment. Now, I see the policy more clearly than before. I will revert the default configuration and add more case-by-case error handling code according to your opinion. Also, I will keep my eyes on this issue.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/433#issuecomment-89548289

        I don't think this issue is trivial.
        You might already consider, but we need to decide which errors are critical or trivial.
        IMO, we can regard the errors caused by users' mistake as trivial ones. For example, getting table meta of non-existing table might be user's mistake.
        Any other errors should be regarded as critical errors, and must be exposed to users to help to find what caused the problems. So, the default configuration must be reverted.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/433#issuecomment-89548289 I don't think this issue is trivial. You might already consider, but we need to decide which errors are critical or trivial. IMO, we can regard the errors caused by users' mistake as trivial ones. For example, getting table meta of non-existing table might be user's mistake. Any other errors should be regarded as critical errors, and must be exposed to users to help to find what caused the problems. So, the default configuration must be reverted.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12707981/TAJO-1413.Hyun.150328.1.patch.txt
        against master revision release-0.9.0-rc0-223-g373d53c.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-cli tajo-common.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/670//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/670//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/670//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707981/TAJO-1413.Hyun.150328.1.patch.txt against master revision release-0.9.0-rc0-223-g373d53c. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-cli tajo-common. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/670//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/670//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/670//console This message is automatically generated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dongjoon-hyun commented on the pull request:

        https://github.com/apache/tajo/pull/433#issuecomment-87191820

        First, change `default setting` to hide stack trace.
        Second, when printing stack tracing. Remove enclosing ServiceException.
        Before
        ```
        default> \d table2
        ERROR: no such a table: table2
        com.google.protobuf.ServiceException: java.sql.SQLException: ERROR: no such a table: table2
        at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:104)
        at org.apache.tajo.client.CatalogAdminClientImpl.getTableDesc(CatalogAdminClientImpl.java:216)
        at org.apache.tajo.client.TajoClientImpl.getTableDesc(TajoClientImpl.java:217)
        at org.apache.tajo.cli.tsql.commands.DescTableCommand.invoke(DescTableCommand.java:50)
        at org.apache.tajo.cli.tsql.TajoCli.executeMetaCommand(TajoCli.java:471)
        at org.apache.tajo.cli.tsql.TajoCli.executeParsedResults(TajoCli.java:445)
        at org.apache.tajo.cli.tsql.TajoCli.runShell(TajoCli.java:419)
        at org.apache.tajo.cli.tsql.TajoCli.main(TajoCli.java:694)
        Caused by: java.sql.SQLException: ERROR: no such a table: table2
        at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:231)
        at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:217)
        at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:94)
        ... 7 more
        ```
        After
        ```
        default> \d table2
        ERROR: no such a table: table2
        java.sql.SQLException: ERROR: no such a table: table2
        at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:231)
        at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:217)
        at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:94)
        at org.apache.tajo.client.CatalogAdminClientImpl.getTableDesc(CatalogAdminClientImpl.java:216)
        at org.apache.tajo.client.TajoClientImpl.getTableDesc(TajoClientImpl.java:217)
        at org.apache.tajo.cli.tsql.commands.DescTableCommand.invoke(DescTableCommand.java:50)
        at org.apache.tajo.cli.tsql.TajoCli.executeMetaCommand(TajoCli.java:471)
        at org.apache.tajo.cli.tsql.TajoCli.executeParsedResults(TajoCli.java:445)
        at org.apache.tajo.cli.tsql.TajoCli.runShell(TajoCli.java:419)
        at org.apache.tajo.cli.tsql.TajoCli.main(TajoCli.java:697)
        ```

        Show
        githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/433#issuecomment-87191820 First, change `default setting` to hide stack trace. Second, when printing stack tracing. Remove enclosing ServiceException. Before ``` default> \d table2 ERROR: no such a table: table2 com.google.protobuf.ServiceException: java.sql.SQLException: ERROR: no such a table: table2 at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:104) at org.apache.tajo.client.CatalogAdminClientImpl.getTableDesc(CatalogAdminClientImpl.java:216) at org.apache.tajo.client.TajoClientImpl.getTableDesc(TajoClientImpl.java:217) at org.apache.tajo.cli.tsql.commands.DescTableCommand.invoke(DescTableCommand.java:50) at org.apache.tajo.cli.tsql.TajoCli.executeMetaCommand(TajoCli.java:471) at org.apache.tajo.cli.tsql.TajoCli.executeParsedResults(TajoCli.java:445) at org.apache.tajo.cli.tsql.TajoCli.runShell(TajoCli.java:419) at org.apache.tajo.cli.tsql.TajoCli.main(TajoCli.java:694) Caused by: java.sql.SQLException: ERROR: no such a table: table2 at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:231) at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:217) at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:94) ... 7 more ``` After ``` default> \d table2 ERROR: no such a table: table2 java.sql.SQLException: ERROR: no such a table: table2 at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:231) at org.apache.tajo.client.CatalogAdminClientImpl$9.call(CatalogAdminClientImpl.java:217) at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:94) at org.apache.tajo.client.CatalogAdminClientImpl.getTableDesc(CatalogAdminClientImpl.java:216) at org.apache.tajo.client.TajoClientImpl.getTableDesc(TajoClientImpl.java:217) at org.apache.tajo.cli.tsql.commands.DescTableCommand.invoke(DescTableCommand.java:50) at org.apache.tajo.cli.tsql.TajoCli.executeMetaCommand(TajoCli.java:471) at org.apache.tajo.cli.tsql.TajoCli.executeParsedResults(TajoCli.java:445) at org.apache.tajo.cli.tsql.TajoCli.runShell(TajoCli.java:419) at org.apache.tajo.cli.tsql.TajoCli.main(TajoCli.java:697) ```
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12705372/TAJO-1413.patch
        against master revision release-0.9.0-rc0-207-ga9ae3ca.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-common.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/620//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/620//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/620//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12705372/TAJO-1413.patch against master revision release-0.9.0-rc0-207-ga9ae3ca. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-common. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/620//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/620//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-common.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/620//console This message is automatically generated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dongjoon-hyun opened a pull request:

        https://github.com/apache/tajo/pull/433

        TAJO-1413: tsql should not print a stacktrace message of not critical exceptions

        Trivial patch.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1413

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/433.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #433


        commit d78d12646ffe7ad8bcfb1e33e5b72667f783747c
        Author: Dongjoon Hyun <dongjoon@apache.org>
        Date: 2015-03-18T16:06:15Z

        TAJO-1413: tsql should not print a stacktrace message of not critical exceptions


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dongjoon-hyun opened a pull request: https://github.com/apache/tajo/pull/433 TAJO-1413 : tsql should not print a stacktrace message of not critical exceptions Trivial patch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongjoon-hyun/tajo TAJO-1413 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/433.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #433 commit d78d12646ffe7ad8bcfb1e33e5b72667f783747c Author: Dongjoon Hyun <dongjoon@apache.org> Date: 2015-03-18T16:06:15Z TAJO-1413 : tsql should not print a stacktrace message of not critical exceptions

          People

          • Assignee:
            dongjoon Dongjoon Hyun
            Reporter:
            hyunsik Hyunsik Choi
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development