Problem
When publishing a dataset to NDP fails for a reason the code does not recognise, the message the user gets names no cause.
api/services/dataset_services/publish_dataset.py handles the failure of preckan_repository.package_create(...) by string-matching on the text of CKAN's error message:
"That name is already in use" / "That URL is already in use" → retries with a timestamp suffix and returns a warning. Works.
"Organization does not exist" → raises ValueError, which the route turns into a 400 with a clear message. Works.
- anything else →
raise Exception(f"Error creating dataset in PRE-CKAN: {error_msg}"), which the route turns into a 500.
The third branch is where the information is lost. error_msg is str(exc), and for the ckanapi exception classes that is often not a message. Measured against the pinned ckanapi in this project's virtualenv:
str(NotAuthorized()) -> 'None'
str(ValidationError({'owner_org': ['Missing value']})) -> "{'owner_org': ['Missing value']}"
So an authorization failure against PRE-CKAN — a wrong or missing PRE_CKAN_API_KEY, the most likely misconfiguration — reaches the user as a 500 reading literally Error creating dataset in PRE-CKAN: None. A validation failure arrives as a raw Python dict. In neither case is the exception class recorded, so the response cannot distinguish an authorization problem from a validation problem from the remote catalog being unreachable.
Matching on English error text is the underlying fragility: any change to CKAN's wording silently demotes a handled case to the generic 500 branch.
Proposed fix
- Include the exception class name alongside its message, so the failure mode is identifiable from the response even when the message is empty.
- Branch on the
ckanapi exception type rather than on the text of its message, keeping the existing text matching only as a fallback for the name-collision case, which is genuinely reported in the message body.
- Map the types to accurate status codes: an authorization failure against PRE-CKAN should not surface as a 500.
Not part of this issue
The bare except Exception: around local_repository.organization_show(...) is not a defect: it guards the resolution of owner_org from a UUID to a name, and falling back to the original value is deliberate and commented. Likewise the swallowed failure when mirroring the submitted status locally, which must not undo a successful PRE-CKAN creation.
Acceptance
- Tests covering each error shape (authorization, validation, connectivity), asserting that the response body names the cause.
CHANGELOG.md entry.
Problem
When publishing a dataset to NDP fails for a reason the code does not recognise, the message the user gets names no cause.
api/services/dataset_services/publish_dataset.pyhandles the failure ofpreckan_repository.package_create(...)by string-matching on the text of CKAN's error message:"That name is already in use"/"That URL is already in use"→ retries with a timestamp suffix and returns a warning. Works."Organization does not exist"→ raisesValueError, which the route turns into a 400 with a clear message. Works.raise Exception(f"Error creating dataset in PRE-CKAN: {error_msg}"), which the route turns into a 500.The third branch is where the information is lost.
error_msgisstr(exc), and for theckanapiexception classes that is often not a message. Measured against the pinnedckanapiin this project's virtualenv:So an authorization failure against PRE-CKAN — a wrong or missing
PRE_CKAN_API_KEY, the most likely misconfiguration — reaches the user as a 500 reading literallyError creating dataset in PRE-CKAN: None. A validation failure arrives as a raw Python dict. In neither case is the exception class recorded, so the response cannot distinguish an authorization problem from a validation problem from the remote catalog being unreachable.Matching on English error text is the underlying fragility: any change to CKAN's wording silently demotes a handled case to the generic 500 branch.
Proposed fix
ckanapiexception type rather than on the text of its message, keeping the existing text matching only as a fallback for the name-collision case, which is genuinely reported in the message body.Not part of this issue
The bare
except Exception:aroundlocal_repository.organization_show(...)is not a defect: it guards the resolution ofowner_orgfrom a UUID to a name, and falling back to the original value is deliberate and commented. Likewise the swallowed failure when mirroring the submitted status locally, which must not undo a successful PRE-CKAN creation.Acceptance
CHANGELOG.mdentry.