{"action":"opened","issue":{"url":"https://api.github.com/repos/pingidentity/ldapsdk/issues/49","repository_url":"https://api.github.com/repos/pingidentity/ldapsdk","labels_url":"https://api.github.com/repos/pingidentity/ldapsdk/issues/49/labels{/name}","comments_url":"https://api.github.com/repos/pingidentity/ldapsdk/issues/49/comments","events_url":"https://api.github.com/repos/pingidentity/ldapsdk/issues/49/events","html_url":"https://github.com/pingidentity/ldapsdk/issues/49","id":354472961,"node_id":"MDU6SXNzdWUzNTQ0NzI5NjE=","number":49,"title":"ldapsdk should stop suppressing `Error`","user":{"login":"jaymode","id":4339958,"node_id":"MDQ6VXNlcjQzMzk5NTg=","avatar_url":"https://avatars1.githubusercontent.com/u/4339958?v=4","gravatar_id":"","url":"https://api.github.com/users/jaymode","html_url":"https://github.com/jaymode","followers_url":"https://api.github.com/users/jaymode/followers","following_url":"https://api.github.com/users/jaymode/following{/other_user}","gists_url":"https://api.github.com/users/jaymode/gists{/gist_id}","starred_url":"https://api.github.com/users/jaymode/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jaymode/subscriptions","organizations_url":"https://api.github.com/users/jaymode/orgs","repos_url":"https://api.github.com/users/jaymode/repos","events_url":"https://api.github.com/users/jaymode/events{/privacy}","received_events_url":"https://api.github.com/users/jaymode/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-08-27T20:49:33Z","updated_at":"2018-08-27T20:49:33Z","closed_at":null,"author_association":"NONE","body":"The ldapsdk currently has exception handling blocks that catch `Throwable` and then anything that is not an `LDAPException`, including `Error` instances, gets wrapped in an `LDAPException`. One such place in the code is: \r\n\r\nhttps://github.com/pingidentity/ldapsdk/blob/0eac394a077323485b850a90e55f926bc035023f/src/com/unboundid/ldap/sdk/LDAPConnectionPool.java#L1623-L1675\r\n\r\nThere are other places in the code where Throwable is caught:\r\n```\r\n$ git rev-parse HEAD\r\n0eac394a077323485b850a90e55f926bc035023f\r\n$ ack --type java \"catch\\s*\\(\\s*(final)?\\s*Throwable\" | wc -l\r\n      43\r\n```\r\n\r\n<details>\r\n$ ack --type java \"catch\\s*\\(\\s*(final)?\\s*Throwable\"\r\ntests/unit/src/com/unboundid/test/LDAPSDKTestListener.java\r\n127:    catch (final Throwable t)\r\n\r\ntests/unit/src/com/unboundid/util/WakeableSleeperTestCaseHelperThread.java\r\n182:      catch (Throwable t)\r\n\r\ntests/unit/src/com/unboundid/ldap/sdk/migrate/ldapjdk/LDAPConnectionTestCase.java\r\n102:    } catch (Throwable t) {}\r\n162:    } catch (Throwable t) {}\r\n377:    } catch (Throwable t) {}\r\n\r\ntests/unit/src/com/unboundid/ldap/sdk/DSEETestCase.java\r\n102:    catch (Throwable t)\r\n\r\nsrc/com/unboundid/util/parallel/ParallelProcessor.java\r\n348:    catch (final Throwable e)\r\n377:    catch (final Throwable e)\r\n\r\nsrc/com/unboundid/util/parallel/AsynchronousParallelProcessor.java\r\n291:        catch (final Throwable e)\r\n\r\nsrc/com/unboundid/ldap/listener/LDAPListenerClientConnection.java\r\n624:        catch (final Throwable t)\r\n\r\nsrc/com/unboundid/ldap/sdk/ConnectThread.java\r\n151:    catch (final Throwable t)\r\n\r\nsrc/com/unboundid/ldap/sdk/AbstractConnectionPool.java\r\n458:    catch (final Throwable t)\r\n472:      catch (final Throwable t2)\r\n540:    catch (final Throwable t)\r\n554:      catch (final Throwable t2)\r\n619:    catch (final Throwable t)\r\n633:      catch (final Throwable t2)\r\n765:    catch (final Throwable t)\r\n779:      catch (final Throwable t2)\r\n869:    catch (final Throwable t)\r\n883:      catch (final Throwable t2)\r\n948:    catch (final Throwable t)\r\n962:      catch (final Throwable t2)\r\n1043:    catch (final Throwable t)\r\n1057:      catch (final Throwable t2)\r\n1181:    catch (final Throwable t)\r\n1196:      catch (final Throwable t2)\r\n1328:    catch (final Throwable t)\r\n1342:      catch (final Throwable t2)\r\n1460:    catch (final Throwable t)\r\n1474:      catch (final Throwable t2)\r\n2060:    catch (final Throwable t)\r\n2083:      catch (final Throwable t2)\r\n2429:    catch (final Throwable t)\r\n2452:      catch (final Throwable t2)\r\n\r\nsrc/com/unboundid/ldap/sdk/LDAPConnectionPool.java\r\n1623:    catch (final Throwable t)\r\n1688:    catch (final Throwable t)\r\n\r\nsrc/com/unboundid/ldap/sdk/persist/DefaultObjectEncoder.java\r\n1251:    catch (final Throwable t)\r\n\r\nsrc/com/unboundid/ldap/sdk/persist/LDAPObjectHandler.java\r\n1233:    catch (final Throwable t)\r\n1302:      catch (final Throwable t)\r\n1401:      catch (final Throwable t)\r\n\r\nsrc/com/unboundid/ldap/sdk/LDAPThreadLocalConnectionPool.java\r\n721:    catch (final Throwable t)\r\n786:    catch (final Throwable t)\r\n</details>\r\n\r\nSome of these cases are in tests but from what I see the majority are not in tests. I did not inspect each case for the behavior of wrapping but I believe each one should be looked at to see if it indeed wraps and hides `Error`s.\r\n\r\nThe Javadocs for `Error` state:\r\n\r\n> An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. \r\n\r\nWhile it is reasonable to catch a `Throwable` and do some cleanup, if the `Throwable` is an `Error` it should not be suppressed by wrapping it in an `LDAPException`. `Error`s should bubble up to the application's handling (if any); in the Elasticsearch case if we hit an error, we want to \"die with dignity\" (see https://github.com/elastic/elasticsearch/pull/19272)."}}
{"actor":{"display_login":"jaymode"}}