NFR gap remediation - error handling & resilience hardening by gkostkowski · Pull Request #13 · OP-TED/entity-resolution-service

gkostkowski · 2026-05-05T13:57:14Z

This change brings in updates and improvements addressing non-functional requirements specified in the PRD.

ServiceUnavailableError commons exception (ers/commons)
Added ServiceUnavailableError(ApplicationError) as a shared exception for infrastructure failures
(MongoDB, Redis, messaging channel). Mapped to HTTP 503 in all API entrypoints.
ERS REST API — error envelope (ers/ers_rest_api)
Renamed the error response field from detail to message and added SERVICE_UNAVAILABLE (503) to the
exception handler chain, registered before the generic ApplicationError handler to preserve MRO.
Curation API — error envelope + SERVICE_UNAVAILABLE (ers/curation)
Standardised the Curation API error shape to {"error_code": "...", "message": "..."} (was {"detail":
"..."}). Replaced the _make_handler factory loop with explicit named handlers. Added
ServiceUnavailableError → 503 and a catch-all Exception → 500 handler. Updated all Curation API tests
to use the new field name.
Resolution Coordinator — connection error propagation (ers/resolution_coordinator)
MongoDB and Redis/channel connection errors during resolve_single now raise ServiceUnavailableError
instead of silently issuing a provisional outcome. Updated unit tests and BDD feature to assert the
new behaviour.
ERE Result Integrator — exponential backoff on reconnect (ers/ere_result_integrator)
Replaced the fixed 5 s reconnect sleep in OutcomeIntegrationWorker with exponential backoff (initial 1
s, cap 30 s, reset on success). Added two unit tests covering consecutive-error doubling and
post-success reset.

…is 0 Both ERS_COORDINATOR_SINGLE_REQUEST_TIME_BUDGET and ERS_COORDINATOR_BULK_REQUEST_TIME_BUDGET now accept 0 as a valid value. When single budget is 0, resolve_single skips ERE submission entirely and issues a provisional identifier without any Redis interaction. When bulk budget is 0, resolve_bulk removes the outer asyncio.wait_for timeout and lets the gather run untimed.

…rovisional mode

…mode

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Python loggers are process-wide singletons; setting propagate=False in create_app() permanently broke pytest caplog for all ers.* loggers across the test session. Records still reach uvicorn handlers via the explicit addHandler call, so Docker log visibility is unaffected.

feat: add immediate provisional mode (ERS_COORDINATOR_*_TIME_BUDGET=0), fix logging bug

Add ServiceUnavailableError subclassing ApplicationError for signalling unreachable backend services (MongoDB, Redis). Also add pythonpath to pytest.ini so the worktree src/ takes precedence over the shared editable install when running tests.

…EST API

…andler

…ailures

…tcomeIntegrationWorker Backoff starts at 1s, doubles on each consecutive ConnectionError, and is capped at 30s. It resets to 1s after any successful message receive.

Copilot

Pull request overview

This PR hardens ERS resilience and error-handling semantics by introducing a shared ServiceUnavailableError for infrastructure outages, standardizing error envelopes across APIs, and tightening coordinator/worker behavior under dependency failures.

Changes:

Added ServiceUnavailableError (mapped to HTTP 503) and propagated it through REST + Curation APIs and the Resolution Coordinator for Mongo/Redis/channel connectivity failures.
Standardized error response bodies to use message (instead of detail) and updated unit/feature/e2e tests accordingly.
Improved runtime resilience: “immediate provisional mode” when single budget is 0, optional removal of bulk outer timeout when bulk budget is 0, and exponential backoff for the outcome integration worker reconnect loop.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/unit/resolution_coordinator/services/test_resolution_coordinator_service.py	Updates coordinator unit tests for 503-on-infra-failure and adds zero-budget mode coverage.
test/unit/ers_rest_api/domain/test_dto_validators.py	Updates DTO validator tests for `ErrorResponse.message`.
test/unit/ers_rest_api/api/test_resolve.py	Updates REST API tests for `message` and adds 503 handler coverage.
test/unit/ers_rest_api/api/test_refresh_bulk.py	Updates validation error assertions to use `message`.
test/unit/ers_rest_api/api/test_lookup.py	Updates validation error assertions to use `message`.
test/unit/ere_result_integrator/entrypoints/test_outcome_integration_worker.py	Adds unit tests for exponential backoff and reset-on-success reconnect logic.
test/unit/curation/api/test_statistics.py	Updates Curation API validation error assertions to use `message`.
test/unit/curation/api/test_exception_handlers.py	Adds new tests covering 503 handling and generic 500 handler behavior in non-debug mode.
test/unit/curation/api/test_decisions.py	Updates Curation API error assertions to use `message`.
test/unit/curation/api/test_auth.py	Updates auth error assertions to use `message`.
test/unit/commons/test_service_unavailable_error.py	Adds unit tests for `ServiceUnavailableError` behavior.
test/unit/commons/adapters/test_app_config.py	Adjusts config singleton test behavior (now conditionally skipped).
test/integration/resolution_coordinator/test_resolution_coordinator_integration.py	Updates integration tests for new infra-failure semantics and adds zero-budget integration scenarios.
test/feature/resolution_coordinator/test_single_mention_resolution.py	Updates BDD steps to support config overrides and new error semantics.
test/feature/resolution_coordinator/single_mention_resolution.feature	Updates scenarios to expect 503-style failures and adds zero-budget scenario.
test/feature/link_curation_api/test_user_management.py	Updates feature assertions from `detail` to `message`.
test/feature/link_curation_api/test_decision_browsing.py	Updates feature assertions from `detail` to `message`.
test/feature/link_curation_api/test_authentication.py	Updates feature assertions from `detail` to `message`.
test/feature/ers_rest_api/test_resolve_entity_mention.py	Updates ERS REST API feature tests to expect `message` field.
test/feature/ers_rest_api/conftest.py	Updates shared feature assertions/docs from `detail` to `message`.
test/e2e/ucs/ucb11_resolve_entity_mention.feature	Updates e2e expectations from 504 timeout to 503 unavailable for dependency failures.
test/e2e/ucs/test_ucb11_resolve_entity_mention.py	Aligns e2e scenario wiring with renamed feature scenarios.
src/pytest.ini	Ensures `pythonpath = .` for test discovery/import behavior under `src/`.
src/ers/resolution_coordinator/services/resolution_coordinator_service.py	Implements infra failures → `ServiceUnavailableError`, supports single/bulk “0 budget” modes.
src/ers/ers_rest_api/services/resolve_service.py	Updates error mapping to `ErrorResponse.message`.
src/ers/ers_rest_api/services/lookup_service.py	Updates error mapping to `ErrorResponse.message`.
src/ers/ers_rest_api/entrypoints/api/exception_handlers.py	Switches envelope to `message` and adds 503 handler for `ServiceUnavailableError`.
src/ers/ers_rest_api/entrypoints/api/app.py	Adds runtime logging notes for 0-budget modes and wires `ers` logger to uvicorn handlers.
src/ers/ers_rest_api/domain/errors.py	Adds `SERVICE_UNAVAILABLE` and renames `detail` → `message` in `ErrorResponse`.
src/ers/ere_result_integrator/entrypoints/outcome_integration_worker.py	Replaces fixed reconnect sleep with exponential backoff and reset-on-success.
src/ers/curation/entrypoints/api/exception_handlers.py	Standardizes Curation API error envelope and adds explicit 503 + 500 catch-all handler.
src/ers/curation/entrypoints/api/app.py	Wires `ers` logger to uvicorn handlers (similar to ERS REST API).
src/ers/curation/domain/errors.py	Introduces Curation API error codes + error DTO.
src/ers/commons/services/exceptions.py	Adds shared `ServiceUnavailableError` exception type.
src/ers/init.py	Documents new 0-budget semantics in config docstrings.
README.md	Documents new coordinator budget knobs and “immediate provisional mode”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

-            raise ResolutionTimeoutError(
+            raise ServiceUnavailableError(
                f"Cannot persist provisional decision: {exc}"
            ) from None


+    _ers_log = logging.getLogger("ers")
+    _ers_log.setLevel(logging.DEBUG if config.DEBUG else logging.INFO)
+    for _h in logging.getLogger("uvicorn").handlers:
+        _ers_log.addHandler(_h)


+    for _h in logging.getLogger("uvicorn").handlers:
+        _ers_log.addHandler(_h)


+_REQUIRED_ENV_VARS = ["JWT_SECRET_KEY", "ADMIN_EMAIL", "ADMIN_PASSWORD"]
+
+
 class TestAppConfigResolverSingleton:
+    @pytest.mark.skipif(
+        not all(os.environ.get(v) for v in _REQUIRED_ENV_VARS),
+        reason=f"Required env vars not set: {', '.join(_REQUIRED_ENV_VARS)}",
+    )
    def test_config_singleton_has_all_keys(self):


                )
                if decision is not None:
                    return decision, ResolutionOutcome.CANONICAL
-            except (TimeoutError, RedisConnectionError, ChannelUnavailableError):
+            except (RedisConnectionError, ChannelUnavailableError) as exc:
+                raise ServiceUnavailableError(str(exc)) from exc
+            except TimeoutError:
                pass


                ) from exc
            return decision, ResolutionOutcome.CANONICAL
        except RepositoryConnectionError as exc:
-            raise ResolutionTimeoutError(
+            raise ServiceUnavailableError(
                f"Cannot persist provisional decision: {exc}"
            ) from None


gkostkowski and others added 20 commits April 28, 2026 16:43

test: add IT-010/IT-011 integration tests for zero-budget immediate p…

8b680f9

…rovisional mode

test: add BDD scenario for immediate provisional mode (budget=0)

21a9cb0

docs: document ERS_COORDINATOR_*_TIME_BUDGET=0 immediate provisional …

70b1488

…mode

Update README.md

f0d4c9e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/ers/__init__.py

b4b499b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

feat: add logging for immediate provisional mode

7fed853

chore: update log message

d3e7cf6

fix: register ers logger

5359987

fix: register ers logger in the curation API

cbae360

Merge pull request #93 from meaningfy-ws/feature/ERS1-229/skip-ere

968ddfd

feat: add immediate provisional mode (ERS_COORDINATOR_*_TIME_BUDGET=0), fix logging bug

chore: remove unused pytest import from test_service_unavailable_error

8baa433

feat: rename error field to message, add SERVICE_UNAVAILABLE to ERS R…

3cf50d5

…EST API

feat: standardise Curation API error shape, add SERVICE_UNAVAILABLE h…

bbf422b

…andler

feat: raise ServiceUnavailableError on MongoDB and Redis connection f…

fd3b43a

…ailures

feat: replace fixed 5s reconnect sleep with exponential backoff in Ou…

946063a

…tcomeIntegrationWorker Backoff starts at 1s, doubles on each consecutive ConnectionError, and is capped at 30s. It resets to 1s after any successful message receive.

test: skip config singleton test when required env vars are absent

e81e26b

fix: update Gherkin tests to align with the recent changes

a8d34cd

gkostkowski requested a review from Copilot May 5, 2026 13:57

Copilot started reviewing on behalf of gkostkowski May 5, 2026 13:58 View session

gkostkowski closed this May 5, 2026

Copilot AI reviewed May 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

NFR gap remediation - error handling & resilience hardening#13

NFR gap remediation - error handling & resilience hardening#13
gkostkowski wants to merge 20 commits into
OP-TED:developfrom
meaningfy-ws:feature/ERS1-213

gkostkowski commented May 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		for _h in logging.getLogger("uvicorn").handlers:
		_ers_log.addHandler(_h)

Uh oh!

Conversation

gkostkowski commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants