search

getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 154 forks source link

Missing error details in Central non-success HTTP response log entries #821

Closed ggalmazor closed 4 years ago

ggalmazor commented 4 years ago

Problem description

A log entry is created every time Briefcase receives a non-success response from Central while pulling or pushing forms. This log entry includes information about the error thanks to a JSON object Central encodes in its responses.

Right now, we're parsing the JSON and storing the message in a CentralErrorMessage class, which gets interpolated into the log entry text. Since the class is missing the toString() method, the log entry doesn't include anything useful for users trying to know why an interaction with Central failed.

Example log file:

2019-10-01 15:49:31,535 [ForkJoinPool-1-worker-3] ERROR o.o.b.p.central.PushToCentralTracker - Push Measles_Campaign_Site_Parent_Survey - Error sending submission 1 of 11 HTTP 400 Bad Request org.opendatakit.briefcase.push.central.CentralErrorMessage@44f942bf

Steps to reproduce the problem

Expected behavior

The log file should include the message Central sends back with the 4xx HTTP response.

Example: 

2019-10-01 15:49:31,535 [ForkJoinPool-1-worker-3] ERROR o.o.b.p.central.PushToCentralTracker - Push Measles_Campaign_Site_Parent_Survey - Error sending submission 1 of 11 HTTP 400 Bad Request Unexpected version value 201910902; The submission could not be accepted because your copy of this form is an outdated version. Please get the latest version and try again.

Other information

The CentralErrorMessage was created to store more information about the error, but currently is storing just a message string, which feels silly and overcomplicated.

A simpler solution could involve deleting the class and making PushToCentralTracker.parseErrorResponse() return just the string to be interpolated. We can always recover this class in case we need any of the other data Central includes in the JSON error details.

florian42 commented 4 years ago

Hi I will gladly help out and make PR later today..

ggalmazor commented 4 years ago

Hi @florian42!

That would be perfect, thanks!

Please, let me know if you need help. I'm around at our #briefcase-code channel at Slack

ggalmazor commented 4 years ago

I'll try to provide you with a form and submissions that would reproduce the issue

ggalmazor commented 4 years ago

@florian42, I've attached a zip with a form&submissions that will reproduce the issue when pushed to a Central server. If don't have access to a Central server, send me an email to ggalmazor@gmail.com and I'll create a user for you in the sandbox server.

getodk-bot commented 4 years ago

Hello @florian42, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!