[jira] [Commented] (CASSANDRA-6947) SocketException being swallowed
[ https://issues.apache.org/jira/browse/CASSANDRA-6947?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13950779#comment-13950779 ] Ding Yuan commented on CASSANDRA-6947: -- I see. Thanks for the feedbacks! > SocketException being swallowed > --- > > Key: CASSANDRA-6947 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6947 > Project: Cassandra > Issue Type: Improvement >Reporter: Ding Yuan > Attachments: trunk-socketExceptions.txt > > > There are a few cases where a SocketException is swallowed. For example: in > the following code snippet in TCustomServerSocket.java, a SocketException is > ignored even though the method declares that TTransportException can be > thrown. > {noformat} > public void listen() throws TTransportException > { > // Make sure not to block on accept > if (serverSocket != null) > { > try > { > serverSocket.setSoTimeout(100); > } > catch (SocketException sx) > { > logger.error("Could not set socket timeout.", sx); > } > } > } > {noformat} > Propose to thrown TTransportException on SocketExceptions. Attaching a patch > against trunk for review. Thanks! -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Created] (CASSANDRA-6947) SocketException being swallowed
Ding Yuan created CASSANDRA-6947: Summary: SocketException being swallowed Key: CASSANDRA-6947 URL: https://issues.apache.org/jira/browse/CASSANDRA-6947 Project: Cassandra Issue Type: Improvement Reporter: Ding Yuan Attachments: trunk-socketExceptions.txt There are a few cases where a SocketException is swallowed. For example: in the following code snippet in TCustomServerSocket.java, a SocketException is ignored even though the method declares that TTransportException can be thrown. {noformat} public void listen() throws TTransportException { // Make sure not to block on accept if (serverSocket != null) { try { serverSocket.setSoTimeout(100); } catch (SocketException sx) { logger.error("Could not set socket timeout.", sx); } } } {noformat} Propose to thrown TTransportException on SocketExceptions. Attaching a patch against trunk for review. Thanks! -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Created] (CASSANDRA-6946) Catching RequestExecutionException instead of Throwable
Ding Yuan created CASSANDRA-6946: Summary: Catching RequestExecutionException instead of Throwable Key: CASSANDRA-6946 URL: https://issues.apache.org/jira/browse/CASSANDRA-6946 Project: Cassandra Issue Type: Improvement Components: Core Reporter: Ding Yuan Priority: Trivial Attachments: trunk-overthrow.txt In CassandraAuthorizer.java, there are a few cases where a Throwable is used to catch RequestExecutionException. Propose to catch the exact exception. Attaching a patch against trunk for review. Thanks. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Updated] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ding Yuan updated CASSANDRA-6656: - Attachment: trunk-6656-v3.txt > Exception logging > - > > Key: CASSANDRA-6656 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 > Project: Cassandra > Issue Type: Improvement > Components: Core, Tools >Reporter: Ding Yuan >Assignee: Ding Yuan >Priority: Trivial > Fix For: 2.1 > > Attachments: trunk-6656-v2.txt, trunk-6656-v3.txt, trunk-6656.txt > > > Reporting a few cases where informative exceptions can be silently swallowed. > Attaching a proposed patch. > = > Case 1 > Line: 95, File: "org/apache/cassandra/utils/Hex.java" > An actual failure in the underlying constructor will be lost. > Propose to log it. > {noformat} > try > { > s = stringConstructor.newInstance(0, c.length, c); > + } > + catch (InvocationTargetException ite) { > + // The underlying constructor failed. Unwrapping the > exception. > + logger.info("Underlying constructor throws exception: ", > ite.getCause()); > } > catch (Exception e) > { > // Swallowing as we'll just use a copying constructor > } > return s == null ? new String(c) : s; > {noformat} > == > = > Case 2 > Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" > The actual cause of comparator error can be lost as it can fail in multiple > locations. > {noformat} > AbstractType comparator = null; > int header = getShortLength(bb); > if ((header & 0x8000) == 0) > { > ByteBuffer value = getBytes(bb, header); > try > { > comparator = TypeParser.parse(ByteBufferUtil.string(value)); > } > catch (Exception e) > { > <--- can fail here > // we'll deal with this below since comparator == null > } > } > else > { > comparator = aliases.get((byte)(header & 0xFF)); > <--- can fail here > } > if (comparator == null) > throw new MarshalException("Cannot find comparator for component > " + i); > {noformat} > Propose to log the exception. > == > = > Case 3 > Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" > Exception ignored in finally. Propose log them with debug or trace. > {noformat} > 232: finally > 233: { > 234: try > 235: { > 236: ssProxy.removeNotificationListener(runner); > 236: ssProxy.removeNotificationListener(runner); > 237: jmxc.removeConnectionNotificationListener(runner); > 238: } > 239: catch (Throwable ignored) {} > 240: } > {noformat} > Similar case is at line 264 in the same file. > == -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13894124#comment-13894124 ] Ding Yuan commented on CASSANDRA-6656: -- Agreed Mikhail Stepura! Attached another one to address your comment. > Exception logging > - > > Key: CASSANDRA-6656 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 > Project: Cassandra > Issue Type: Improvement > Components: Core, Tools >Reporter: Ding Yuan >Assignee: Ding Yuan >Priority: Trivial > Fix For: 2.1 > > Attachments: trunk-6656-v2.txt, trunk-6656-v3.txt, trunk-6656.txt > > > Reporting a few cases where informative exceptions can be silently swallowed. > Attaching a proposed patch. > = > Case 1 > Line: 95, File: "org/apache/cassandra/utils/Hex.java" > An actual failure in the underlying constructor will be lost. > Propose to log it. > {noformat} > try > { > s = stringConstructor.newInstance(0, c.length, c); > + } > + catch (InvocationTargetException ite) { > + // The underlying constructor failed. Unwrapping the > exception. > + logger.info("Underlying constructor throws exception: ", > ite.getCause()); > } > catch (Exception e) > { > // Swallowing as we'll just use a copying constructor > } > return s == null ? new String(c) : s; > {noformat} > == > = > Case 2 > Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" > The actual cause of comparator error can be lost as it can fail in multiple > locations. > {noformat} > AbstractType comparator = null; > int header = getShortLength(bb); > if ((header & 0x8000) == 0) > { > ByteBuffer value = getBytes(bb, header); > try > { > comparator = TypeParser.parse(ByteBufferUtil.string(value)); > } > catch (Exception e) > { > <--- can fail here > // we'll deal with this below since comparator == null > } > } > else > { > comparator = aliases.get((byte)(header & 0xFF)); > <--- can fail here > } > if (comparator == null) > throw new MarshalException("Cannot find comparator for component > " + i); > {noformat} > Propose to log the exception. > == > = > Case 3 > Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" > Exception ignored in finally. Propose log them with debug or trace. > {noformat} > 232: finally > 233: { > 234: try > 235: { > 236: ssProxy.removeNotificationListener(runner); > 236: ssProxy.removeNotificationListener(runner); > 237: jmxc.removeConnectionNotificationListener(runner); > 238: } > 239: catch (Throwable ignored) {} > 240: } > {noformat} > Similar case is at line 264 in the same file. > == -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13892919#comment-13892919 ] Ding Yuan commented on CASSANDRA-6656: -- Thanks Mikhail. Updated the patch. > Exception logging > - > > Key: CASSANDRA-6656 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 > Project: Cassandra > Issue Type: Improvement > Components: Core, Tools >Reporter: Ding Yuan >Assignee: Ding Yuan >Priority: Trivial > Fix For: 2.1 > > Attachments: trunk-6656-v2.txt, trunk-6656.txt > > > Reporting a few cases where informative exceptions can be silently swallowed. > Attaching a proposed patch. > = > Case 1 > Line: 95, File: "org/apache/cassandra/utils/Hex.java" > An actual failure in the underlying constructor will be lost. > Propose to log it. > {noformat} > try > { > s = stringConstructor.newInstance(0, c.length, c); > + } > + catch (InvocationTargetException ite) { > + // The underlying constructor failed. Unwrapping the > exception. > + logger.info("Underlying constructor throws exception: ", > ite.getCause()); > } > catch (Exception e) > { > // Swallowing as we'll just use a copying constructor > } > return s == null ? new String(c) : s; > {noformat} > == > = > Case 2 > Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" > The actual cause of comparator error can be lost as it can fail in multiple > locations. > {noformat} > AbstractType comparator = null; > int header = getShortLength(bb); > if ((header & 0x8000) == 0) > { > ByteBuffer value = getBytes(bb, header); > try > { > comparator = TypeParser.parse(ByteBufferUtil.string(value)); > } > catch (Exception e) > { > <--- can fail here > // we'll deal with this below since comparator == null > } > } > else > { > comparator = aliases.get((byte)(header & 0xFF)); > <--- can fail here > } > if (comparator == null) > throw new MarshalException("Cannot find comparator for component > " + i); > {noformat} > Propose to log the exception. > == > = > Case 3 > Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" > Exception ignored in finally. Propose log them with debug or trace. > {noformat} > 232: finally > 233: { > 234: try > 235: { > 236: ssProxy.removeNotificationListener(runner); > 236: ssProxy.removeNotificationListener(runner); > 237: jmxc.removeConnectionNotificationListener(runner); > 238: } > 239: catch (Throwable ignored) {} > 240: } > {noformat} > Similar case is at line 264 in the same file. > == -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Updated] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ding Yuan updated CASSANDRA-6656: - Attachment: trunk-6656-v2.txt > Exception logging > - > > Key: CASSANDRA-6656 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 > Project: Cassandra > Issue Type: Improvement > Components: Core, Tools >Reporter: Ding Yuan >Assignee: Ding Yuan >Priority: Trivial > Fix For: 2.1 > > Attachments: trunk-6656-v2.txt, trunk-6656.txt > > > Reporting a few cases where informative exceptions can be silently swallowed. > Attaching a proposed patch. > = > Case 1 > Line: 95, File: "org/apache/cassandra/utils/Hex.java" > An actual failure in the underlying constructor will be lost. > Propose to log it. > {noformat} > try > { > s = stringConstructor.newInstance(0, c.length, c); > + } > + catch (InvocationTargetException ite) { > + // The underlying constructor failed. Unwrapping the > exception. > + logger.info("Underlying constructor throws exception: ", > ite.getCause()); > } > catch (Exception e) > { > // Swallowing as we'll just use a copying constructor > } > return s == null ? new String(c) : s; > {noformat} > == > = > Case 2 > Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" > The actual cause of comparator error can be lost as it can fail in multiple > locations. > {noformat} > AbstractType comparator = null; > int header = getShortLength(bb); > if ((header & 0x8000) == 0) > { > ByteBuffer value = getBytes(bb, header); > try > { > comparator = TypeParser.parse(ByteBufferUtil.string(value)); > } > catch (Exception e) > { > <--- can fail here > // we'll deal with this below since comparator == null > } > } > else > { > comparator = aliases.get((byte)(header & 0xFF)); > <--- can fail here > } > if (comparator == null) > throw new MarshalException("Cannot find comparator for component > " + i); > {noformat} > Propose to log the exception. > == > = > Case 3 > Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" > Exception ignored in finally. Propose log them with debug or trace. > {noformat} > 232: finally > 233: { > 234: try > 235: { > 236: ssProxy.removeNotificationListener(runner); > 236: ssProxy.removeNotificationListener(runner); > 237: jmxc.removeConnectionNotificationListener(runner); > 238: } > 239: catch (Throwable ignored) {} > 240: } > {noformat} > Similar case is at line 264 in the same file. > == -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Updated] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ding Yuan updated CASSANDRA-6656: - Description: Reporting a few cases where informative exceptions can be silently swallowed. Attaching a proposed patch. = Case 1 Line: 95, File: "org/apache/cassandra/utils/Hex.java" An actual failure in the underlying constructor will be lost. Propose to log it. {noformat} try { s = stringConstructor.newInstance(0, c.length, c); + } + catch (InvocationTargetException ite) { + // The underlying constructor failed. Unwrapping the exception. + logger.info("Underlying constructor throws exception: ", ite.getCause()); } catch (Exception e) { // Swallowing as we'll just use a copying constructor } return s == null ? new String(c) : s; {noformat} == = Case 2 Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" The actual cause of comparator error can be lost as it can fail in multiple locations. {noformat} AbstractType comparator = null; int header = getShortLength(bb); if ((header & 0x8000) == 0) { ByteBuffer value = getBytes(bb, header); try { comparator = TypeParser.parse(ByteBufferUtil.string(value)); } catch (Exception e) { <--- can fail here // we'll deal with this below since comparator == null } } else { comparator = aliases.get((byte)(header & 0xFF)); <--- can fail here } if (comparator == null) throw new MarshalException("Cannot find comparator for component " + i); {noformat} Propose to log the exception. == = Case 3 Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" Exception ignored in finally. Propose log them with debug or trace. {noformat} 232: finally 233: { 234: try 235: { 236: ssProxy.removeNotificationListener(runner); 236: ssProxy.removeNotificationListener(runner); 237: jmxc.removeConnectionNotificationListener(runner); 238: } 239: catch (Throwable ignored) {} 240: } {noformat} Similar case is at line 264 in the same file. == was: Reporting a few cases where informative exceptions can be silently swallowed. Attaching a proposed patch. = Case 1 Line: 95, File: "org/apache/cassandra/utils/Hex.java" An actual failure in the underlying constructor will be lost. Propose to log it. try { s = stringConstructor.newInstance(0, c.length, c); + } + catch (InvocationTargetException ite) { + // The underlying constructor failed. Unwrapping the exception. + logger.info("Underlying constructor throws exception: ", ite.getCause()); } catch (Exception e) { // Swallowing as we'll just use a copying constructor } return s == null ? new String(c) : s; == = Case 2 Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" The actual cause of comparator error can be lost as it can fail in multiple locations. AbstractType comparator = null; int header = getShortLength(bb); if ((header & 0x8000) == 0) { ByteBuffer value = getBytes(bb, header); try { comparator = TypeParser.parse(ByteBufferUtil.string(value)); } catch (Exception e) { <--- can fail here // we'll deal with this below since comparator == null } } else { comparator = aliases.get((byte)(header & 0xFF)); <--- can fail here } if (comparator == null) throw new MarshalException("Cannot find comparator for component " + i); Propose to log the exception. == = Case 3 Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" Exception ignored in finally. Propose log them with debug or trace. 232: finally 233: { 234: try 235: { 236: ssProxy.removeNotificationListener(runner); 236: ssProxy.removeNotificationListener(runner); 237: jmxc.remo
[jira] [Updated] (CASSANDRA-6656) Exception logging
[ https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ding Yuan updated CASSANDRA-6656: - Attachment: trunk-6656.txt > Exception logging > - > > Key: CASSANDRA-6656 > URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 > Project: Cassandra > Issue Type: Improvement > Components: Core, Tools >Reporter: Ding Yuan > Fix For: 2.0.4 > > Attachments: trunk-6656.txt > > > Reporting a few cases where informative exceptions can be silently swallowed. > Attaching a proposed patch. > = > Case 1 > Line: 95, File: "org/apache/cassandra/utils/Hex.java" > An actual failure in the underlying constructor will be lost. > Propose to log it. > try > { > s = stringConstructor.newInstance(0, c.length, c); > + } > + catch (InvocationTargetException ite) { > + // The underlying constructor failed. Unwrapping the > exception. > + logger.info("Underlying constructor throws exception: ", > ite.getCause()); > } > catch (Exception e) > { > // Swallowing as we'll just use a copying constructor > } > return s == null ? new String(c) : s; > == > = > Case 2 > Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" > The actual cause of comparator error can be lost as it can fail in multiple > locations. > AbstractType comparator = null; > int header = getShortLength(bb); > if ((header & 0x8000) == 0) > { > ByteBuffer value = getBytes(bb, header); > try > { > comparator = TypeParser.parse(ByteBufferUtil.string(value)); > } > catch (Exception e) > { > <--- can fail here > // we'll deal with this below since comparator == null > } > } > else > { > comparator = aliases.get((byte)(header & 0xFF)); > <--- can fail here > } > if (comparator == null) > throw new MarshalException("Cannot find comparator for component > " + i); > Propose to log the exception. > == > = > Case 3 > Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" > Exception ignored in finally. Propose log them with debug or trace. > 232: finally > 233: { > 234: try > 235: { > 236: ssProxy.removeNotificationListener(runner); > 236: ssProxy.removeNotificationListener(runner); > 237: jmxc.removeConnectionNotificationListener(runner); > 238: } > 239: catch (Throwable ignored) {} > 240: } > Similar case is at line 264 in the same file. > == -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Created] (CASSANDRA-6656) Exception logging
Ding Yuan created CASSANDRA-6656: Summary: Exception logging Key: CASSANDRA-6656 URL: https://issues.apache.org/jira/browse/CASSANDRA-6656 Project: Cassandra Issue Type: Improvement Components: Core, Tools Reporter: Ding Yuan Fix For: 2.0.4 Reporting a few cases where informative exceptions can be silently swallowed. Attaching a proposed patch. = Case 1 Line: 95, File: "org/apache/cassandra/utils/Hex.java" An actual failure in the underlying constructor will be lost. Propose to log it. try { s = stringConstructor.newInstance(0, c.length, c); + } + catch (InvocationTargetException ite) { + // The underlying constructor failed. Unwrapping the exception. + logger.info("Underlying constructor throws exception: ", ite.getCause()); } catch (Exception e) { // Swallowing as we'll just use a copying constructor } return s == null ? new String(c) : s; == = Case 2 Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java" The actual cause of comparator error can be lost as it can fail in multiple locations. AbstractType comparator = null; int header = getShortLength(bb); if ((header & 0x8000) == 0) { ByteBuffer value = getBytes(bb, header); try { comparator = TypeParser.parse(ByteBufferUtil.string(value)); } catch (Exception e) { <--- can fail here // we'll deal with this below since comparator == null } } else { comparator = aliases.get((byte)(header & 0xFF)); <--- can fail here } if (comparator == null) throw new MarshalException("Cannot find comparator for component " + i); Propose to log the exception. == = Case 3 Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java" Exception ignored in finally. Propose log them with debug or trace. 232: finally 233: { 234: try 235: { 236: ssProxy.removeNotificationListener(runner); 236: ssProxy.removeNotificationListener(runner); 237: jmxc.removeConnectionNotificationListener(runner); 238: } 239: catch (Throwable ignored) {} 240: } Similar case is at line 264 in the same file. == -- This message was sent by Atlassian JIRA (v6.1.5#6160)