-
Notifications
You must be signed in to change notification settings - Fork 0
Expose structured OperationOutcome issues on client errors ( python and typescript ) #384
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| import json | ||
| import os | ||
| from ctypes import ArgumentError | ||
| from dataclasses import dataclass | ||
| from typing import Any, Mapping, Optional, Union | ||
|
|
||
| import requests | ||
| from orchestrate._internal.exceptions import ( | ||
| OperationOutcomeIssue, | ||
| OrchestrateClientError, | ||
| OrchestrateHttpError, | ||
| ) | ||
|
|
@@ -60,29 +60,12 @@ def _get_additional_headers() -> Mapping[str, str]: | |
| ) | ||
|
|
||
|
|
||
| @dataclass | ||
| class _OperationalOutcomeIssue: | ||
| severity: str | ||
| code: str | ||
| diagnostics: str | ||
| details: str | ||
|
|
||
| def __str__(self) -> str: | ||
| s = f"{self.severity}: {self.code}" | ||
| message = "; ".join( | ||
| message for message in [self.details, self.diagnostics] if message | ||
| ) | ||
| if message: | ||
| s += f" - {message}" | ||
| return s | ||
|
|
||
|
|
||
| def _read_json_outcomes(response: requests.Response) -> list[_OperationalOutcomeIssue]: | ||
| def _read_json_outcomes(response: requests.Response) -> list[OperationOutcomeIssue]: | ||
| try: | ||
| json_response = response.json() | ||
| if "issue" in json_response: | ||
| return [ | ||
|
Comment on lines
+63
to
67
|
||
| _OperationalOutcomeIssue( | ||
| OperationOutcomeIssue( | ||
| issue.get("severity", ""), | ||
| issue.get("code", ""), | ||
| issue.get("diagnostics", ""), | ||
|
|
@@ -95,7 +78,7 @@ def _read_json_outcomes(response: requests.Response) -> list[_OperationalOutcome | |
| == "https://tools.ietf.org/html/rfc9110#section-15.5.1" | ||
| ): | ||
| return [ | ||
| _OperationalOutcomeIssue( | ||
| OperationOutcomeIssue( | ||
| severity="error", | ||
| code=json_response.get("title", ""), | ||
| diagnostics=json_response.get("detail", ""), | ||
|
|
@@ -122,18 +105,10 @@ def _get_detail_coding_string(coding: dict) -> str: | |
| ) | ||
|
|
||
|
|
||
| def _read_operational_outcomes(response: requests.Response) -> list[str]: | ||
| outcomes = _read_json_outcomes(response) | ||
| if outcomes: | ||
| return [str(outcome) for outcome in outcomes] | ||
|
|
||
| return [response.text] | ||
|
|
||
|
|
||
| def _exception_from_response(response: requests.Response) -> OrchestrateHttpError: | ||
| operational_outcomes = _read_operational_outcomes(response) | ||
| issues = _read_json_outcomes(response) | ||
| if response.status_code >= 400 and response.status_code < 600: | ||
| return OrchestrateClientError(response.text, operational_outcomes) | ||
| return OrchestrateClientError(response.text, issues, response.status_code) | ||
| return OrchestrateHttpError() | ||
|
Comment on lines
108
to
112
|
||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import json | ||
| from unittest.mock import Mock | ||
|
|
||
| import pytest | ||
| from orchestrate._internal.exceptions import OrchestrateClientError | ||
| from orchestrate._internal.http_handler import _exception_from_response | ||
|
|
||
| pytestmark = pytest.mark.default | ||
|
|
||
| _CDA_TO_FHIR_OPERATION_OUTCOME = { | ||
| "resourceType": "OperationOutcome", | ||
| "issue": [ | ||
| { | ||
| "severity": "error", | ||
| "code": "invalid", | ||
| "diagnostics": "Missing recordTarget in ClinicalDocument", | ||
| }, | ||
| { | ||
| "severity": "information", | ||
| "code": "informational", | ||
| "details": { | ||
| "coding": [ | ||
| { | ||
| "system": "https://quality.rosetta.careevolution.com/v1/CodeSystems/CDAProcessingMessage", | ||
| "code": "DocumentId", | ||
| } | ||
| ], | ||
| "text": "fb04306a-0834-432d-90c3-251ed7d3401d", | ||
| }, | ||
| }, | ||
| { | ||
| "severity": "information", | ||
| "code": "informational", | ||
| "details": { | ||
| "coding": [ | ||
| { | ||
| "system": "https://quality.rosetta.careevolution.com/v1/CodeSystems/CDAProcessingMessage", | ||
| "code": "DocumentEffectiveTime", | ||
| } | ||
| ], | ||
| "text": "2011-05-27T01:44:27-05:00", | ||
| }, | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
|
|
||
| def _make_mock_response(payload: dict, status_code: int) -> Mock: | ||
| response = Mock() | ||
| response.status_code = status_code | ||
| response.text = json.dumps(payload) | ||
| response.json.return_value = payload | ||
| return response | ||
|
|
||
|
|
||
| def test_multiple_issues_should_all_be_captured(): | ||
| response = _make_mock_response(_CDA_TO_FHIR_OPERATION_OUTCOME, 400) | ||
|
|
||
| exc = _exception_from_response(response) | ||
|
|
||
| assert isinstance(exc, OrchestrateClientError) | ||
| assert len(exc.issues) == 3 | ||
|
|
||
|
|
||
| def test_error_issue_with_diagnostics_only(): | ||
| response = _make_mock_response(_CDA_TO_FHIR_OPERATION_OUTCOME, 400) | ||
|
|
||
| exc = _exception_from_response(response) | ||
|
|
||
| assert isinstance(exc, OrchestrateClientError) | ||
| issue = exc.issues[0] | ||
| assert issue.severity == "error" | ||
| assert issue.code == "invalid" | ||
| assert issue.diagnostics == "Missing recordTarget in ClinicalDocument" | ||
| assert issue.details == "" | ||
|
|
||
|
|
||
| def test_information_issue_with_details_text(): | ||
| response = _make_mock_response(_CDA_TO_FHIR_OPERATION_OUTCOME, 400) | ||
|
|
||
| exc = _exception_from_response(response) | ||
|
|
||
| assert isinstance(exc, OrchestrateClientError) | ||
| doc_id_issue = exc.issues[1] | ||
| assert doc_id_issue.severity == "information" | ||
| assert doc_id_issue.code == "informational" | ||
| assert doc_id_issue.details == "fb04306a-0834-432d-90c3-251ed7d3401d" | ||
| assert doc_id_issue.diagnostics == "" | ||
|
|
||
| effective_time_issue = exc.issues[2] | ||
| assert effective_time_issue.details == "2011-05-27T01:44:27-05:00" | ||
|
|
||
|
|
||
| def test_status_code_and_response_text_are_preserved(): | ||
| response = _make_mock_response(_CDA_TO_FHIR_OPERATION_OUTCOME, 400) | ||
|
|
||
| exc = _exception_from_response(response) | ||
|
|
||
| assert isinstance(exc, OrchestrateClientError) | ||
| assert exc.status_code == 400 | ||
| assert exc.response_text == response.text | ||
|
|
||
|
|
||
| def test_message_string_includes_all_issues(): | ||
| response = _make_mock_response(_CDA_TO_FHIR_OPERATION_OUTCOME, 400) | ||
|
|
||
| exc = _exception_from_response(response) | ||
|
|
||
| message = str(exc) | ||
| assert "Missing recordTarget in ClinicalDocument" in message | ||
| assert "fb04306a-0834-432d-90c3-251ed7d3401d" in message | ||
| assert "2011-05-27T01:44:27-05:00" in message |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,16 +12,44 @@ export class OrchestrateHttpError extends OrchestrateError { | |||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export class OrchestrateClientError extends OrchestrateError { | ||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(response_text: string, issues: string[]) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| let message; | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (issues.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| message = `\n * ${issues.join(" \n * ")}`; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||||||||||||||||
| message = response_text; | ||||||||||||||||||||||||||||||||||||||||||||||||
| export class OperationOutcomeIssue { | ||||||||||||||||||||||||||||||||||||||||||||||||
| severity: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| code: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| diagnostics: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| details: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(severity: string, code: string, diagnostics: string, details: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.severity = severity; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.code = code; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.diagnostics = diagnostics; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.details = details; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| toString(): string { | ||||||||||||||||||||||||||||||||||||||||||||||||
| let s = `${this.severity}: ${this.code}`; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const message = [this.details, this.diagnostics] | ||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(msg => msg) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .join("; "); | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (message) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| s += ` - ${message}`; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| return s; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export class OrchestrateClientError extends OrchestrateError { | ||||||||||||||||||||||||||||||||||||||||||||||||
| responseText: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| issues: OperationOutcomeIssue[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| statusCode: number; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(responseText: string, issues: OperationOutcomeIssue[], statusCode: number = 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const message = issues.length > 0 | ||||||||||||||||||||||||||||||||||||||||||||||||
| ? `\n * ${issues.map(i => i.toString()).join(" \n * ")}` | ||||||||||||||||||||||||||||||||||||||||||||||||
| : responseText; | ||||||||||||||||||||||||||||||||||||||||||||||||
| super(message); | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.name = this.constructor.name; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.responseText = responseText; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.issues = issues; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+52
|
||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(responseText: string, issues: OperationOutcomeIssue[], statusCode: number = 0) { | |
| const message = issues.length > 0 | |
| ? `\n * ${issues.map(i => i.toString()).join(" \n * ")}` | |
| : responseText; | |
| super(message); | |
| this.name = this.constructor.name; | |
| this.responseText = responseText; | |
| this.issues = issues; | |
| constructor(responseText: string, issues: OperationOutcomeIssue[], statusCode?: number); | |
| constructor(responseText: string, issues: string[], statusCode?: number); | |
| constructor(responseText: string, issues: OperationOutcomeIssue[] | string[], statusCode: number = 0) { | |
| const normalizedIssues = issues.map(issue => | |
| typeof issue === "string" | |
| ? new OperationOutcomeIssue("error", "unknown", issue, "") | |
| : issue | |
| ); | |
| const message = normalizedIssues.length > 0 | |
| ? `\n * ${normalizedIssues.map(i => i.toString()).join(" \n * ")}` | |
| : responseText; | |
| super(message); | |
| this.name = this.constructor.name; | |
| this.responseText = responseText; | |
| this.issues = normalizedIssues; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codeis just one of the values from https://build.fhir.org/valueset-issue-type.htmlI think we need to know the detail.coding as well?