hapifhir / hapi-fhir-jpaserver-starter

Apache License 2.0
394 stars 1.05k forks source link

fix intermittent error with CDS test #753

Closed XcrigX closed 3 weeks ago

XcrigX commented 4 weeks ago

I was getting an error running the CdsHooksServletIT.testCdsHooks() test on Windows. The test is blocking while it waits for the CDS Services to be "ready" by looking at the response from /cds-services. I was seeing an empty response from it initially like:

{
  "services": []
}

This empty response is 22 characters (on Windows), which is the minimum size that hasCdsServices() was looking for. So the test was continuing to run thinking the CDS services were "ready", but they were not.

This led to:

2024-10-29T12:58:34.199-05:00  INFO 19928 --- [           main] c.u.h.f.c.svc.CdsServiceRegistryImpl     : Unregistered active service opioidcds-10-order-sign
2024-10-29T12:58:34.436-05:00  INFO 19928 --- [o-auto-1-exec-2] fhirtest.access                          : Path[/fhir] Source[] Operation[transaction  ] UA[HAPI-FHIR/7.4.0 (FHIR Client; FHIR 4.0.1/R4; apache)] Params[] ResponseEncoding[JSON] Operation[transaction  ] UA[HAPI-FHIR/7.4.0 (FHIR Client; FHIR 4.0.1/R4; apache)] Params[] ResponseEncoding[JSON]
2024-10-29T12:58:34.542-05:00  INFO 19928 --- [o-auto-1-exec-4] c.u.f.j.s.cdshooks.CdsHooksServlet       : /cds-services
2024-10-29T12:58:34.546-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : /cds-services/hello-world
2024-10-29T12:58:34.549-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : {  "hookInstance": "12345",  "hook": "patient-view",  "context": {    "userId": "Practitioner/example",    "patientId": "Patient/example-hello-world"  },  "prefetch": {    "item1": {      "resourceType": "Patient",      "id": "example-hello-world",      "gender": "male",      "birthDate": "2000-01-01"    }  }}
2024-10-29T12:58:34.549-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : cds-hooks hook instance: 12345
2024-10-29T12:58:34.549-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : cds-hooks local server address: null
2024-10-29T12:58:34.549-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : cds-hooks fhir server address: null
2024-10-29T12:58:34.550-05:00  INFO 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : cds-hooks cql_logging_enabled: false
2024-10-29T12:58:34.553-05:00 ERROR 19928 --- [o-auto-1-exec-6] c.u.f.j.s.cdshooks.CdsHooksServlet       : ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException: HAPI-2391: No service with id hello-world is registered on this server

com.google.gson.JsonSyntaxException: Expected a com.google.gson.JsonObject but was com.google.gson.JsonPrimitive

    at com.google.gson.internal.bind.TypeAdapters$33$1.read(TypeAdapters.java:869)
    at com.google.gson.Gson.fromJson(Gson.java:963)
    at com.google.gson.Gson.fromJson(Gson.java:928)
    at com.google.gson.Gson.fromJson(Gson.java:877)
    at com.google.gson.Gson.fromJson(Gson.java:848)
    at ca.uhn.fhir.jpa.starter.CdsHooksServletIT.testCdsHooks(CdsHooksServletIT.java:139)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

You can see just before the exception it is saying: "HAPI-2391: No service with id hello-world is registered on this server" Changing the test response size to >22 causes it to wait for a non-empty response from /cds-services and the test then passes.

NOTE: This original test passed for me on a linux VM. It obviously runs here in github okay. But it was failing consistently on my Windows machine. Perhaps it's because on Linux the empty response is shorter than 22 chars due to line endings being different sizes?

XcrigX commented 3 weeks ago

Hi @dotasek - I don't disagree with you that the test could be designed better, I just know that it was failing on Windows (for me anyway), so I updated it to pass. My theory is that linux/mac use a 1-character line ending, so testing for >22 worked to indicate a non-empty response. Windows uses a 2-character line ending so a non-empty response is slightly longer.

XcrigX commented 3 weeks ago

.. and to your point, if you try to actually look at the string it consumes the steam which then causes the test to fail. I didn't dig in enough to determine how to get the string out and not then fail the test later - so that it could test for specific contents in the string rather than just the length.

dotasek commented 3 weeks ago

Yeah, it sounds like we both ran into the same issues. I'm just being nitpicky here. If this fixes things for Windows, and doesn't impact other systems, this is fine.

Maybe a comment in the code would be appropriate?

XcrigX commented 3 weeks ago

I'll add some comments there and push

XcrigX commented 3 weeks ago

comments added