Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added POST search capability for prefetch queries with long URLs #155

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

c-schuler
Copy link
Contributor

This is usually handled by the receiving FHIR server when the URL is too long, but not always.

This is usually handled by the receiving FHIR server when the URL is too long, but not always.
@c-schuler
Copy link
Contributor Author

@jmandel Please review when possible. Thanks!

@jmandel
Copy link
Member

jmandel commented Nov 7, 2024

Thanks! Could you include in the discussion here an explanation of what the problem is, where you've encountered it, and what the fix is?

(Generally a FHIR server can support queries via GET, POST, or both... but a client needs to choose a server-supported method.)

@c-schuler
Copy link
Contributor Author

@jmandel Absolutely.

The issue stems from prefetch templates that result in long urls that are not supported through GET requests.

For example, navigate to the following scenario:
https://sandbox.cds-hooks.org/?fhirServiceUrl=https://opioid-sandbox.cqframework.org/cdc/opioid-cds-r4/ehr/fhir&serviceDiscoveryURL=https://opioid-sandbox.cqframework.org/cdc/opioid-cds-r4/cds-service/cds-services&patientId=example-rec-10-order-sign-illicit-POS-Cocaine-drugs&prescribedMedication=857007&prescribedSupplyDuration=30&screen=rx-sign

When you click the Order Sign button, you'll find errors in the console stating that the sandbox was unable to prefetch the data due to a 400 error (which is actually a URI too long error). You can see the prefetch template in the Configure CDS Services menu for the opioidcds-10-order-sign service includes rather long urls. The underlying FHIR server (https://opioid-sandbox.cqframework.org/cdc/opioid-cds-r4/ehr/fhir) is an out-of-the-box HAPI FHIR server. It was my understanding that GET requests with urls of that length would be converted to POST requests when encountered, but that hasn't been my experience in practice. Therefore, I have provided the logic to convert the prefetch requests to use POST when the url is greater than 2000 characters.

I am happy to employ a better solution! Please let me know if you have any questions or concerns.

Thanks!

@jmandel
Copy link
Member

jmandel commented Nov 8, 2024

Thanks -- that explanation (and the deep link to reproduce!) are extremely helpful.

Your approach here works fine, but I'm slightly concerned about the separation of responsibilities. Ultimately the CDS Hooks Sandbox is configured "to act as if" it was associated with a given FHIR endpoint, and it uses that FHIR endpoint to prepare prefetch responses for CDS Clients. The fhirServiceUrl param controls which FHIR endpoint to "bake in" to a given sandbox session.

Different FHIR endpoints might have different behaviors. For example, https://opioid-sandbox.cqframework.org/cdc/opioid-cds-r4/ehr/fhir supports POST for any size of request, and supports GET for "short enough" requests. But another server might support only POST, and another server might support only GET.

So if it makes sense to you, I'd rather direct the Sandbox more explicitly, e.g. with something like fhirServiceSearchMethod=GET|POST param that controls the sandbox behavior in a given session. (Or potentially a test requests at the beginning of a session to determine if POST is supported -- and if so, always use POST; if not, always use GET.)

@c-schuler
Copy link
Contributor Author

c-schuler commented Nov 8, 2024

@jmandel Thank you for the feedback! I agree that your approach makes more sense. I have updated the prefetch resolution to attempt search by POST first and then by GET if that results in an error. I will look to add a param to specify the strategy more explicitly in future PRs. I believe what I have implemented should serve well as default behavior.

@jmandel jmandel merged commit 6a0a974 into cds-hooks:master Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants