Skip to content

An unrecognised PRE-CKAN failure is reported as "Error creating dataset in PRE-CKAN: None" #263

Description

@rbardaji

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions