PUT /api/dandisets/{id}/versions/{version}/ returns incorrect 405 for non-draft versions

[waxlamp]

Description

In VersionViewSet.update() (dandiapi/api/views/version.py), when a PUT request targets a non-draft version, the handler returns HTTP 405 with a plain string body:

if version.version != 'draft':
    return Response(
        'Only draft versions can be modified.',
        status=status.HTTP_405_METHOD_NOT_ALLOWED,
    )

There are two problems with this:

1. Wrong status code

HTTP 405 Method Not Allowed has a specific meaning: the HTTP method (here, PUT) is not supported on the target URL. It should be accompanied by an Allow header listing valid methods. That's not the situation here — PUT is perfectly valid on this URL; it just can't be applied to a published version's current state. A more appropriate status code would be:

• 409 Conflict — the request conflicts with the current state of the resource, or
• 422 Unprocessable Entity — the request is well-formed but cannot be acted upon

2. Inconsistent response body

All other errors from this endpoint go through DRF's exception machinery and return a standard JSON envelope:

{"detail": "..."}

This response returns a raw string instead, which breaks any client code that parses errors uniformly.

Contrast

The very next guard in the same method raises a proper DRF-compatible exception:

if version.dandiset.unembargo_in_progress:
    raise DandisetUnembargoInProgressError  # → 400, JSON body

Suggested fix

Replace the return Response(...) with a raised DRF exception (e.g. ValidationError or a custom DandiError subclass) using an appropriate status code, so the response shape is consistent with the rest of the API.

[jjnesbitt]

First off, I agree with your point that we should be raising DandiError exceptions instead of returning the response directly. However, I want to address a few things about this.

1. Wrong status code

I don't think that 422 is a better response code here than 405. In fact, I think 405 Method Not Allowed is a perfectly valid response here. The PUT method on published dandiset versions is not allowed, but other methods are (HEAD and GET).

HTTP 405 Method Not Allowed has a specific meaning: the HTTP method (here, PUT) is not supported on the target URL. It should be accompanied by an Allow header listing valid methods. That's not the situation here — PUT is perfectly valid on this URL; it just can't be applied to a published version's current state.

I understand what you're saying here, but I disagree with it. PUT is not perfectly valid on this URL, that being /dandisets/123456/versions/0.260218.2052 (as an example). The PUT method is never allowed on published versions, it is only allowed on draft versions, which would be a different resource and URL (/dandisets/123456/versions/draft).

To your point about returning the Allow header, I agree that if we're going to return a 405 Method Not Allowed, we should include this header. I think this is an opportunity to improve the way we handle errors, and include a way to set headers in the raised exception.

2. Inconsistent response body

All other errors from this endpoint go through DRF's exception machinery and return a standard JSON envelope:

{"detail": "..."}

This is actually not true. Only DRF-based exceptions return a response in this format. Here is where we handle exceptions and wrap them in a response. An easy way to try this for yourself is to attempt to create a dandiset with the same identifier as an existing one. The response you get comes from a raised exception, but is simply the following (this is an example response from me doing just this on dandiset 000003):

"Dandiset 000003 already exists"

It would probably be fairly straightforward to change the handling of DandiErrors to match that of DRF, but regardless, it's not something limited to just this endpoint.

---

This is actually making me realize that there are some other inconsistencies in the version view set along these lines.

For one, the publish endpoint returns a 405, when in reality that should be some other 4xx level error, since the error with the request isn't the method (there are no other methods allowed at that endpoint, only POST), it's the fact that trying to publish a published version is nonsensical. Realistically, this endpoint should live at the dandiset level, since the action of publishing only ever applies to the singular draft dandiset. There's only one allowed version value draft, which becomes superfluous.

Another one is the version destroy endpoint, where we return a 403 when trying to destroy a draft version. I think this is actually another spot where 405 Method Not Allowed is the correct choice, as that URL is shared with several other methods that are valid (HEAD, GET and PUT).

I think this is an opportunity to improve the behavior of the version viewset endpoints overall. There are probably plenty of other places in the codebase where this is the case, however.

[waxlamp]

I don't think that 422 is a better response code here than 405. In fact, I think 405 Method Not Allowed is a perfectly valid response here. The PUT method on published dandiset versions is not allowed, but other methods are (HEAD and GET).

HTTP 405 Method Not Allowed has a specific meaning: the HTTP method (here, PUT) is not supported on the target URL. It should be accompanied by an Allow header listing valid methods. That's not the situation here — PUT is perfectly valid on this URL; it just can't be applied to a published version's current state.

I understand what you're saying here, but I disagree with it. PUT is not perfectly valid on this URL, that being /dandisets/123456/versions/0.260218.2052 (as an example). The PUT method is never allowed on published versions, it is only allowed on draft versions, which would be a different resource and URL (/dandisets/123456/versions/draft).

You're right, I agree that 405 is appropriate here, since it's the resource specifically that doesn't support a PUT operation (and I've updated #2743 to reflect that).

As for your point 2, we can file an issue to improve error handling throughout, but is my proposed change in #2743 still an improvement?

[jjnesbitt]

Yes, I think that change is still an improvement.