Bugsnag issue - Glific

[mahalakshme]

Issue:

https://app.bugsnag.com/samanvay-research-and-development-foundation/rwb/errors/65f418424354650008ddd406?event_id=65f5706800e02d92e0b90000&i=em&m=fq - Issue in RWB environment - 100 events occurred.

java.lang.NullPointerException
    String.java:2240 java.lang.String.replace
    GlificContactRepository.java:85 org.avni.messaging.repository.GlificContactRepository.createContact
    GlificContactRepository.java:75 org.avni.messaging.repository.GlificContactRepository.getOrCreateContact
    <generated>:-1 org.avni.messaging.repository.GlificContactRepository$$FastClassBySpringCGLIB$$91f1d5e8.invoke
    MethodProxy.java:204 org.springframework.cglib.proxy.MethodProxy.invoke
    CglibAopProxy.java:747 org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint
    ReflectiveMethodInvocation.java:163 org.springframework.aop.framework.ReflectiveMethodInvocation.proceed
    PersistenceExceptionTranslationInterceptor.java:139 org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke
    ReflectiveMethodInvocation.java:185 org.springframework.aop.framework.ReflectiveMethodInvocation.proceed
    CglibAopProxy.java:689 org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept
    <generated>:-1 org.avni.messaging.repository.GlificContactRepository$$EnhancerBySpringCGLIB$$735a656.getOrCreateContact
    MessageReceiverService.java:75 org.avni.messaging.service.MessageReceiverService.ensureExternalIdPresent
    IndividualMessagingService.java:42 org.avni.messaging.service.IndividualMessagingService.ensureExternalIdPresenceAndSendMessage
    IndividualMessagingService.java:56 org.avni.messaging.service.IndividualMessagingService.sendAutomatedMessage
    MessagingService.java:174 org.avni.messaging.service.MessagingService.sendMessageToGlific
    MessagingService.java:133 org.avni.messaging.service.MessagingService.sendMessage
    Iterator.java:116 java.util.Iterator.forEachRemaining
    Spliterators.java:1801 java.util.Spliterators$IteratorSpliterator.forEachRemaining
    ReferencePipeline.java:647 java.util.stream.ReferencePipeline$Head.forEach
    MessagingService.java:157 org.avni.messaging.service.MessagingService.sendMessages
    <generated>:-1 org.avni.messaging.service.MessagingService$$FastClassBySpringCGLIB$$a16061e8.invoke
    MethodProxy.java:204 org.springframework.cglib.proxy.MethodProxy.invoke
    CglibAopProxy.java:747 org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint
    ReflectiveMethodInvocation.java:163 org.springframework.aop.framework.ReflectiveMethodInvocation.proceed
    TransactionAspectSupport.java:294 org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction
    TransactionInterceptor.java:98 org.springframework.transaction.interceptor.TransactionInterceptor.invoke
    ReflectiveMethodInvocation.java:185 org.springframework.aop.framework.ReflectiveMethodInvocation.proceed
    CglibAopProxy.java:689 org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept
    <generated>:-1 org.avni.messaging.service.MessagingService$$EnhancerBySpringCGLIB$$23fb8318.sendMessages
    MessageSenderJob.java:57 org.avni.messaging.service.MessageSenderJob.sendMessages
    MessageSenderJob.java:46 org.avni.messaging.service.MessageSenderJob.sendMessages
    Unknown:-1 sun.reflect.GeneratedMethodAccessor389.invoke
    DelegatingMethodAccessorImpl.java:43 sun.reflect.DelegatingMethodAccessorImpl.invoke
    Method.java:498 java.lang.reflect.Method.invoke
    ScheduledMethodRunnable.java:65 org.springframework.scheduling.support.ScheduledMethodRunnable.run
    DelegatingErrorHandlingRunnable.java:54 org.springframework.scheduling.support.DelegatingErrorHandlingRunnable.run
    Executors.java:511 java.util.concurrent.Executors$RunnableAdapter.call
    FutureTask.java:308 java.util.concurrent.FutureTask.runAndReset
    ScheduledThreadPoolExecutor.java:180 java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301
    ScheduledThreadPoolExecutor.java:294 java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run
    ThreadPoolExecutor.java:1149 java.util.concurrent.ThreadPoolExecutor.runWorker
    ThreadPoolExecutor.java:624 java.util.concurrent.ThreadPoolExecutor$Worker.run
    Thread.java:750 java.lang.Thread.run

Acceptance criteria:

• Identify the root cause and make fix if needed in the 7.1.0 branch
• We will do the release to RWB env after discussion if necessary

As of 20th March AC:

• For users who dont have name, add a migration to insert before the @ as name of users
• Update last_modified_date_time in message_request_queue to make sure the users who didnt receive the message, receives the message
• Identify who were the users, who uploaded the CSV without 'Full Name of User' column entered. This is needed to communicate to the client.

[1t5j0y]

Issue is due to null names provided for users (probably via csv upload). Webapp ui and save already check for null names. DB level change introduced in 7.1 to make name not null as part of avniproject/avni-client#1238. Tested to check that this blocks csv uploads with null names from going through.

Thus deploying 7.1 as it is will prevent this issue from happening.

However, the error message while uploading csv with no name column is not very user friendly:

"could not execute statement; SQL [n/a]; constraint [name]; nested exception is org.hibernate.exception.ConstraintViolationException: could not execute statement"

If the name column is included in the csv but value is left blank for a row, the name is set to empty string.

To discuss if any further handling is required:

• Make name @NotNull in User.java
• Improve error messaging for csv upload failure
• Change any of the above handling.

[mahalakshme]

@1t5j0y I think we can do the below things:

• If user's name is not there, then username can be used - Request you to make sure if this is already handled via CSV in previous build deployed to RWB env
• Update the message_request_queue's last_modified_date_time so that the pending messages go through since we have configured to make it to send messages only for last 4 days - Request you to make sure if this is needed from the DB data.
• Release this to RWB prod - along with if any other fixes needed - not in scope of this card.
• The other cases of user name handling via CSV I think can be done in one of the subsequent releases we are working currently.

[1t5j0y]

Alternatively, communicate to client to set names for users with missing names.

[mahalakshme]

@1t5j0y I discussed with the implementation team and also checked the DB and Glific webconsole, Found the below:

• Glific is used not just to send messages, also a user flow has been setup for user to communicate back.
• All users have not yet started using the app
• 73 users have not received the message
• Sandip or sometimes from client side - upload the users CSV

So I suggest to do the below fix now: Do the below on priority:

• For users who dont have name, add a migration to insert before the @ as name of users
• Update last_modified_date_time in message_request_queue to make sure those 73 users receive the message

After the above is done, do the below in 6.x branch to be able to release to RWB: When username is missing in CSV, we show this error message "Invalid username ''. It must be at least 7 characters.". Similarly when 'Full Name of User' column is missing, show a message in CSV stating 'Full Name of User column cannot be empty'

[1t5j0y]

Don't agree with the above course of action.

This is easily fixable by user communication (to provide names for users while uploading csv and to fix users already created for whom names have not been provided) and a data patch (if user welcome messages for these users absolutely do need to be sent out).

The requirement for infrequent releases does not gel well with releasing patch fixes for things that can be fixed by other means.

Moving this back to hold till we discuss this.

[mahalakshme]

I still think the above fixes are needed since it is not the same set of users who upload CSV always. And also since this season RWB is going to scale, it is out of our control to make sure if the needful is communicated to everyone who will be uploading the CSV.

I think we atleast need to do the below: we might need to add a DB trigger to fill the name of the user(from username), everytime it is empty when inserting into DB.

@vinayvenu let us know the way forward on this.

[mahalakshme]

@1t5j0y I talked with the client to understand the importance of patch fix since Vinay was also not sure. He mentioned he will make sure from his side to upload CSVs with name. But from the conversation, I realised below is needed. Can you kindly do the below:

• For users who dont have name, add a migration to insert before the @ as name of users
• Update last_modified_date_time in message_request_queue to make sure the users who didnt receive the message, receives the message
• Identify who was the users, who uploaded the CSV without 'Full Name of User' of the users column entered.

It is also difficult for me to understand the priority since understanding of the priority of this in itself is different across different people within our company. So may be the only solution is to check the client.

[1t5j0y]

Sounds like this can be handled by support or implementation team.