Properly handle 404s

[davmlaw]

This was found because when DEBUG=False, the APPEND_SLASH doesn't work correctly.

This is because I added a custom handler, in urls we have:

handler404 = views.page_not_found

This calls:

variantgrid.views.custom_error_view which returns a 500

[davmlaw]

testing, vg test, http://test.variantgrid.com/admin now works

[davmlaw]

I think we need to test this in Shariant test, as we have custom error pages there, so test should also be that a garbage URL (eg "/admin4123123") fails properly as well

[EmmaTudini]

@davmlaw What should this do? What does fail properly mean? In prod vs test - if I input /classification/export12, it results in the same thing No slack 404 message and a "could not determine page" message

[TheMadBug]

This one is very technical, if we go to a made up URL in prod we get a 500 (generic server error)

If we do it in test, we get a proper 404 page not found

Having a 404 then lets Django attempt to try another match, specifically by adding a / to the end of the URL.

When this behaviour was broken going to /admin caused an error, but now if you go to /admin it creates a 404 behind the scenes and it will then try /admin/ which works.

[EmmaTudini]

Did a quick test of this with the 28941sd after allele in prod and test, and verified that prod when to a 500 error, and test to 404 error - Passed

Also did some other tests to ensure that links were still abiding security permissions.

• Logged in as emmatudini+8@gmail.com (non-admin) to Shariant test
• view_allele/23651 - Record unshared with user - Passed – allele page with no records classification/36308- Record unshared with user- Passed – Forbidden error

[EmmaTudini]

@TheMadBug Are we supposed to get errors in slack about the 404 error?

[TheMadBug]

So we do get errors in rollbar for 404s

but they are warnings specifically with the wording "PermissionDenied" in them, which we specifically don't report on Slack (I assume due to excessive number of them at some point).

I'll remove the special case for the reporting of them, in test, demo and prod. If we start getting spammed by the messages, we can re-evaluate.

[EmmaTudini]

So even me searching for a weird URL, doesn't cause a slack error - view_allele/28941sd

[TheMadBug]

Note... yup, just about to say the above. I'm not sure as to why that's not reporting, that said, we'll probably get hit by a bunch of requests from Russia of shariant.org.au/admin.php multiple times a day if those are sent.

Still I'd like to know why it's not since it seems to be configured to send it - though maybe Rollbar's reporting of 404 only happens when you manually raise a 404 (like we do when we can't find an object for an ID) and not when the URL doesn't match any known patterns.

[TheMadBug]

Looking at the documention for Rollbar, catching of 404s needs to be the first thing in the order of processes, while catching regular errors should be near the end.

We have our Rollbar catcher near the end, so if we add rollbar.contrib.django.middleware.RollbarNotifierMiddlewareOnly404 near the start that would probably catch them. Might save that for a new issue as prod doesn't currently report them, and I'd rather avoid changing the "middleware" at this stage so close to release.

[TheMadBug]

Handling of 404s is now in #3643, though given the above is the spam traffic we get, just from India, for half a day - if we reported every 404 our rollbar would explode. The upside is we do have messages for well formed URLs but with IDs that don't exist or the user doesn't have access to

[EmmaTudini]

@TheMadBug Can this be moved to pending release? Does #3643 need to happen before this deploy?

[TheMadBug]

Moving to pending release. I don't feel #3643 should be done at all, but just moved it out for separate discussion.