Graceful handling of filter input errors

[torgeirl]

Multiple filter functions will raise an InvalidFiltersStringError if the input isn't «a valid filter slug». These errors doesn't provide any actionable data, and provide two issues:

1. in some rare instances we have to take extra steps when handling these errors since they contain user inputs
2. the errors clutter our error tracking

These issues will potentially grow when we emigrate to another infrastructure, and we should therefore look at addressing the underlying issues in advance.

Some «offenders»:

• /devilry_admin/assignment/{roleid}/examinerdetails/{relatedexaminer_id}
• /devilry_admin/assignment/{roleid}/studentoverview/filter/{filters_string}
• /devilry_admin/period/{roleid}/admins/add/{filters_string}
• /devilry_admin/period/{roleid}/overview/filter/{filters_string}
• /devilry_admin/studentfeedbackfeedwizard/user-filter/{filters_string}
• /devilry_examiner/assignment/{roleid}/bulk-feedback-points-filter/filter/{filters_string}
• /devilry_examiner/assignment/{roleid}/filter/{filters_string}
• /devilry_student/allperiods/{filters_string}
• /devilry_student/period/{roleid}/overview/{filters_string}

All of them seems to be raised from cradmin_legacy/viewhelpers/listfilter/base/filtershandler.py.

Edit: added /devilry_admin/period/{roleid}/overview/filter/{filters_string} to the list on August 14, 2024.

[Levijatan]

It looks like this error should only be raised when there is an invalid filter in the url. This is also the only way I have been able to trigger it. I think that it would better to ignore invalid filters rather then raising an error. It is not the input that is not «a valid filter slug» it is the name of the filter, so as long as filters are implemented correct there should be no user input in the error though if the url is logged it is right there.

[torgeirl]

if the url is logged it is right there

Yes, services like Sentry will store the URL as part of the error.

In the same way that we can implement filter for Sentry's transactions to mask a webhook hash from being passed to Sentry it is probably possible to alter the these URLs from the error messages before they passed on to the Sentry server, but errors gets passed on to other systems as well and it just seems better to handle this issue «at the source».

[Levijatan]

Pushed up a fix for this in c09408e. Made a custom FiltersHandler that overrides parse to change it from
if slug not in self.filtermap: raise InvalidFiltersStringError('"{}" is not a valid filter slug.'.format(slug)) self.filtermap[slug].set_values(values)
to
if slug in self.filtermap: self.filtermap[slug].set_values(values)
So it ignores invalid filters instead of raising an error.

[espenak]

This has been fixed in cradmin_legacy instead (in appressoas/cradmin_legacy@64576a2), but update of that is blocked by Django5 (cradmin_legacy and all the dependencies is updated for django5 already), so @torgeirl can we move this to another milestone?

[torgeirl]

@espenak: yeah, let do this alongside #1319 in v6.5!