Skip to content

CSV export raises bare ValueError, which reaches the caller as a 500 #844

Description

@bencap

ValueError has no registered exception handler, so it falls through to the catch-all in server_main.py. The caller gets {"detail": "Internal server error"} with a correlation id, and send_slack_error fires. Two paths in the CSV package raise it for conditions that are not server faults.

Column-name collision

assemble_csv_headers (src/mavedb/lib/csv/columns.py) raises when two namespaces resolve to the same header. Only the un-namespaced callers can collide, since namespaced output keeps the prefix that separates them.

Nothing in validate_column_names (src/mavedb/lib/validation/dataframe/dataframe.py) reserves the core column names, so an investigator may upload a score column named accession, hgvs_nt, hgvs_splice, or hgvs_pro. GET /score-sets/{urn}/scores then requests ["scores", "scores_custom"] un-namespaced, the core column and the investigator's column both resolve to the same header, and every download of that score set 500s.

The corpus sweep does not cover this: export_sweep._compose_score_set_csv composes with namespaced=True, which always prefixes.

Variant deleted mid-request

get_variant_csv and available_variant_csv_namespaces (src/mavedb/lib/csv/variant.py) raise ValueError(f"variant with URN '{urn}' not found"). The routers check existence first and 404, so this fires only if the variant is deleted between that check and the lib call. Rare, but it produces a 500 and a Slack alert where a 404 is the correct answer.

Work

  • Give the CSV package a typed exception per case and register handlers in server_main.py: 404 for the missing variant, and for a collision whichever status the decision below settles on.
  • Decide what a collision should do on GET /score-sets/{urn}/scores. The request is not at fault there and the score set is otherwise downloadable, so a 422 makes a legitimately published score set undownloadable through that endpoint. Disambiguating the colliding column with its namespace prefix keeps the download working but varies the published header. Either is defensible; pick one and state it in assemble_csv_headers.
  • Consider reserving the core column names in validate_column_names so the collision cannot be created by a new upload. This does not help score sets that already carry such a column.

Acceptance criteria

  • A score set with a score column named accession, downloaded through GET /score-sets/{urn}/scores, returns the chosen status rather than 500.
  • get_variant_csv raising for a missing variant surfaces as 404 through GET /variants/{urn}/csv and GET /variants/{urn}/csv-namespaces.
  • No path in src/mavedb/lib/csv/ raises bare ValueError to a router.

Metadata

Metadata

Assignees

No one assigned

    Labels

    app: backendTask implementation touches the backendneeds: discussionBlocked on a team decision before work can proceed

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions