-1, there are multiple STOMP regressions here

1) In some of my tests that verify error handling works properly, I'm
getting a NPE here that causes my build to fail:
https://github.com/apache/activemq/blob/activemq-5.15.12/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java#L261

The root cause is there's no guarantee there will be a linked exception and
in this case the linked exception is null leading to an NPE

2) It makes no sense to swallow the exception here and not propagate it
up:
https://github.com/apache/activemq/blob/3c302dce33500cd8e36f626af333ce208ebc44a0/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/StompNIOSSLTransport.java#L67

Every other protocol throws the IOException and this no longer does.
Furthermore, it makes no sense to treat STOMP different than other
protocols.  I don't see why we need to log at all when the IOException
should propagate up and then be logged elsewhere.  If it's not being logged
properly then something else up the chain (TcpTransport or whatever) should
probably be catching the exception and logging.

On Mon, Mar 9, 2020 at 8:33 AM Daniel Kulp <[email protected]> wrote:

> +1
>
> Dan
>
>
> > On Mar 6, 2020, at 2:15 AM, Jean-Baptiste Onofre <[email protected]>
> wrote:
> >
> > Hi everyone,
> >
> > I'm submitting ActiveMQ 5.15.12 release to your vote.
> >
> > This release includes dependency updates (especially for CVE/Security
> > fixes), important improvements on JDBC persistence adapter, other
> improvements and fixes.
> >
> > Please take a look on the Release Notes for details:
> >
> >
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
> <
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12346500
> >
> >
> > The Maven staging repository is:
> >
> https://repository.apache.org/content/repositories/orgapacheactivemq-1204
> >
> > The dist staging repository is:
> > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/ <
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.12/>
> >
> > Git tag:
> > activemq-5.15.12
> >
> > Website PR:
> > https://github.com/apache/activemq-website/pull/28 <
> https://github.com/apache/activemq-website/pull/28>
> >
> > Please vote to approve this release:
> >
> > [ ] +1 Approve the release
> > [ ] -1 Don't approve the release (please provide specific comments)
> >
> > This vote will be open for at least 72 hours.
> >
> > Thanks !
> > Regards
> > JB
>
> --
> Daniel Kulp
> [email protected] <mailto:[email protected]> - http://dankulp.com/blog <
> http://dankulp.com/blog>
> Talend Community Coder - http://talend.com <http://coders.talend.com/>
>

Reply via email to