fix: report the real status from handle_error instead of 405

[blarghmatey]

What are the relevant tickets?

N/A

Description (What does it do?)

main/views.py:handle_error is registered as Django's handler400/handler403/handler404, but was decorated with a bare @api_view(). DRF defaults that to GET-only, so any non-GET request raising PermissionDenied, BadRequest/SuspiciousOperation, or Http404 was answered 405 Method Not Allowed, Allow: GET, OPTIONS with error_type: MethodNotAllowed, and the real reason never reached the caller.

• Spell out every method in @api_view([...]) so the handler is reachable at all.
• Map the exception to its real status: PermissionDenied → 403, BadRequest/SuspiciousOperation → 400, otherwise 404.
• Set error_type explicitly per branch rather than from type(exception).__name__ — an unmatched URL raises Resolver404, which would have silently changed the published error_type from "Http404".
• webhooks/views_test.py::test_ovs_video_webhook_invalid_signature asserted 405 for an invalid signature, encoding the bug as expected behaviour; it now asserts 403.

This is API-wide, but the webhooks are where it did damage: an unsigned POST to a real webhook route and a POST to a nonexistent URL returned byte-identical 405s, which is what made the Canvas ContentFile failure look like a routing problem for ten days (~5,500 events on DAGSTER-13) while the real cause — a null course_readable_id rejected by the serializer — sat in the app logs. That payload bug is already fixed by #3776 and mitodl/ol-data-platform#2567; this fixes the masking that hid it.

How can this be tested?

main/views_test.py adds coverage for each status the handler now distinguishes: an unmatched /api/v1/ route is 404 for POST/PUT/PATCH/DELETE (previously 405), a signature failure is 403, and a serializer failure is 400.

What I ran locally against Postgres:

pytest webhooks/ main/ website_content/admin_test.py
→ 291 passed, 1 skipped

pytest channels/ authentication/ content_feedback/
→ 12 failures, all PermissionError: /var/media in
  channels/serializers_test.py (unwritable local MEDIA_ROOT, unrelated)

pre-commit run --files main/views.py main/views_test.py webhooks/views_test.py is clean.

To confirm by hand, POST to a webhook endpoint without a signature header — it should now return 403 PermissionDenied rather than 405 MethodNotAllowed.

[github-actions]

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

[Copilot]

Pull request overview

Corrects Django error handling so non-GET requests report their actual 400/403/404 status instead of 405.

Changes:

• Enables the shared handler for common HTTP methods.
• Maps client exceptions to accurate statuses and error types.
• Adds regression coverage for webhook and unmatched-route errors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
main/views.py	Preserves actual client-error statuses.
main/views_test.py	Tests error handling across methods and statuses.
webhooks/views_test.py	Updates invalid-signature expectations to 403.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ChristopherChudzicki]

Non-blocking: — this is better than what's on main, so if you want to merge it, sure. But rather than an explicit list of methods and isinstance-ing errors, I think a better approach is:

1. Drop the DRF api_view and permission_classes decorators entirely
2. Use separate handle_40x methods.

Why: handle_40x is invoked only when DRF does NOT handle the exception itself and Django already knows the error type.

• Raised inside a DRF view and DRF recognises it (APIException, Http404, PermissionDenied) -> handle_exception -> main.exceptions.api_exception_handler builds the response. handler40x is never called.
• Everything else -> Django's response_for_exception -> handler40x.

So by the time handle_error runs, DRF has already passed on this one. @api_view cannot recover a status DRF didn't set; it can only add DRF's request pipeline — authentication, CSRF, throttling, content negotiation, NamespaceVersioning, method dispatch — to a request that has already proven it isn't DRF's to handle.

That second bucket is also much wider than unmatched URLs, which is why the blast radius was bigger than it looked:

• Before any view — URL-resolution Http404, SuspiciousOperation/DisallowedHost from middleware.
• Non-DRF views — Django admin, oauth2_provider.
• DRF views where the exception escapes dispatch — require_signature is stacked on dispatch via method_decorator, so its PermissionDenied fires outside APIView.dispatch's try/except.
• DRF views where DRF declines the exception — ContentFileWebhookView.get_data raises django.core.exceptions.BadRequest, which isn't an APIException, so handle_exception re-raises it.

As best I can tell, handle_error was introduced on an incorrect premise¹.

Instead

def _error(status_code, error_type, detail):
    return JsonResponse({"detail": detail, "error_type": error_type}, status=status_code)


def handle_400(request, exception=None):  # noqa: ARG001
    return _error(400, "BadRequest", "The request could not be processed.")


def handle_403(request, exception=None):  # noqa: ARG001
    return _error(403, "PermissionDenied", "You do not have permission to perform this action.")


def handle_404(request, exception=None):  # noqa: ARG001
    return _error(404, "Http404", "The specified resource was not found.")

Comparison

Same probes against this branch and against the three handlers above, locally with DEBUG=False:

Request	This branch	Three handlers
POST unmatched /api/v1/...	404 Http404	404 Http404
POST webhook, no signature	403 PermissionDenied	403 PermissionDenied
GET unmatched, Accept: text/html	404, browsable-API HTML page	404 JSON
GET unmatched, Accept: application/xml	406 NotAcceptable	404 JSON
multipart POST with no boundary	404 {"detail": "Invalid version in URL path...", "error_type": "NotFound"}	400 BadRequest

That last row is handler400, on a resolved URL, answering 404 with a versioning error. /admin/, /o/ and /scim/v2/ all carry namespaces outside ALLOWED_VERSIONS, so determine_version raises NotFound from initial() before the body ever runs.²

Costs

Costs, in case any of them are wanted:

• Browser requests to a bad URL stop getting the browsable-API 404 page; every Django-level error becomes JSON. (mit-learn doesn't set DEFAULT_RENDERER_CLASSES, so BrowsableAPIRenderer is currently live in production.)
• website_content/admin_test.py goes 404 -> 403, because versioning no longer short-circuits — which makes Copilot's second comment right after all, and lets the versioning explanation come out of that test rather than get reworded.
• "PermissionDenied" duplicates main.constants.PERMISSION_DENIED_ERROR_TYPE, which is referenced nowhere in the repo — worth either using or deleting, independently of this.

main/ webhooks/ website_content/admin_test.py passes against the split — 302 passed, 1 skipped, with the admin_test.py assertion flipped to 403. Happy to push the branch if that's easier than reworking this one.

Footnotes

1. "This is a generic handler, since the api_view decorator means DRF will usurp error handling and provide whatever response is actually necessary." (Code comment) ↩

2. determine_version returns default_version when there is no namespace at all, so it is specifically namespaces that exist but are not v0/v1 that raise — admin, oauth2_provider, ol-scim, scim. ↩

[blarghmatey]

Adopted in 95d4ff4: handle_error is now three plain handle_400/403/404 functions, no @api_view/permission_classes. Updated website_content/admin_test.py's assertion to 403 per your versioning analysis (DRF's request pipeline is gone, so it no longer short-circuits under /admin/). Left PERMISSION_DENIED_ERROR_TYPE in place and now actually used by handle_403. Full main/ webhooks/ website_content/admin_test.py suite: 303 passed, 1 skipped.