NullPointerException on reading the error stream

[yantantether]

We occasionally hit a NPE when using the Java Gov Notify client.

It happens on conn.getErrorStream() when the HTTP response code is not as expected. We don't know the underlying cause of why the stream is not available, perhaps a socket error.

It would helpful to have a little more protection around lines 322-329 of uk.gov.service.notify.NotificationClient.

Here is the stack trace:

java.lang.NullPointerException: null
    at java.io.Reader.<init>(Reader.java:78)
    at java.io.InputStreamReader.<init>(InputStreamReader.java:113)
    at uk.gov.service.notify.NotificationClient.performPostRequest(NotificationClient.java:327)
    at uk.gov.service.notify.NotificationClient.sendSms(NotificationClient.java:192)
    at uk.gov.service.notify.NotificationClient.sendSms(NotificationClient.java:171)
    at uk.nhs.nhsbsa.covid19.faster.testing.service.GovNotifyService.sendSms(GovNotifyService.java:110)
    at uk.nhs.nhsbsa.covid19.faster.testing.service.GovNotifyService.send(GovNotifyService.java:62)
    at uk.nhs.nhsbsa.covid19.faster.testing.listener.FasterTestingQueueListener.lambda$listen$0(FasterTestingQueueListener.java:47)
    at java.util.Optional.map(Optional.java:215)
    at uk.nhs.nhsbsa.covid19.faster.testing.listener.FasterTestingQueueListener.listen(FasterTestingQueueListener.java:47)
    at sun.reflect.GeneratedMethodAccessor102.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.messaging.handler.invocation.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:171)
    at org.springframework.messaging.handler.invocation.InvocableHandlerMethod.invoke(InvocableHandlerMethod.java:120)
    at org.springframework.messaging.handler.invocation.AbstractMethodMessageHandler.handleMatch(AbstractMethodMessageHandler.java:565)
    at org.springframework.messaging.handler.invocation.AbstractMethodMessageHandler.handleMessageInternal(AbstractMethodMessageHandler.java:520)
    at org.springframework.messaging.handler.invocation.AbstractMethodMessageHandler.handleMessage(AbstractMethodMessageHandler.java:454)
    at org.springframework.cloud.aws.messaging.listener.SimpleMessageListenerContainer.executeMessage(SimpleMessageListenerContainer.java:227)
    at org.springframework.cloud.aws.messaging.listener.SimpleMessageListenerContainer$MessageExecutor.run(SimpleMessageListenerContainer.java:417)
    at org.springframework.cloud.aws.messaging.listener.SimpleMessageListenerContainer$SignalExecutingRunnable.run(SimpleMessageListenerContainer.java:309)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

[idavidmcdonald]

Hi @yantantether,

Sorry for the delay in getting back to you on this. We aren't java experts but that seems like a reasonable suggestion but it's unlikely we will get round to it too soon I'm afraid. Would you be interested/able to put in a pull request for this?

[yantantether]

Yes, I can try to get a PR together

[idavidmcdonald]

Thanks! Check out our CONTRIBUTING.md and .github/PULL_REQUEST_TEMPLATE.md files for what we would ideally be looking for. Just shout if you need any help or guidance, we are very happy to help.

Thanks for your time once again!

[grantforrester-nes]

I have created a PR that addresses this issue #177 . To properly unit test this I would have to do some refactoring of NotificationClient.java. I'll see if I get the time.

[servingUpAces]

Thank you for your contribution. The changes you made are now available on Bintray.