Error handling: how to properly prioritize exceptions

Question

There are two functions: one downloads the excel file (ExcelFileUploadView(APIView)) and the other processes the downloaded file(def parse_excel_rfi_sheet). Function parse_excel_rfi_sheet is called inside ExcelFileUploadView(APIView)

class ExcelFileUploadView(APIView):
    parser_classes = (MultiPartParser, FormParser)
    permission_classes = (permissions.AllowAny,)

    def put(self, request, format=None):
        if 'file' not in request.data:
            raise ParseError("Empty content")
        f = request.data['file']
        filename = f.name
        if filename.endswith('.xlsx'):
            try:
                file = default_storage.save(filename, f)
                r = parse_excel_rfi_sheet(file)
                status = 200
            except:
                raise Exception({"general_errors": ["Error during file upload"]})
            finally:
                default_storage.delete(file)
        else:
            status = 406
            r = {"general_errors": ["Please upload only xlsx files"]}
        return Response(r, status=status) 

def parse_excel_rfi_sheet(file):
    workbook = load_workbook(filename=file)
    sheet = workbook["RFI"]
    curent_module_coordinate = []
    try:
        ....
        curent_module_coordinate.append(sheet['E688'].value)  
        curent_module_coordinate.append(sheet['E950'].value)  
        if check_exel_rfi_template_structure(structure=curent_module_coordinate):
            file_status = True
        else:
            file_status = False
    except:
        raise Exception({"general_errors": ["Error during excel file parsing. Unknown module cell"]})

The problem is that when an error occurs inside the parse_excel_rfi_sheet, I do not see a call of {"general_errors": ["Error during excel file parsing. Unknown module cell"]} Instead, I always see the call

{"general_errors": ["Error during file upload"]}

That's why I can't understand at what stage the error occurred: at the moment of downloading the file or at the moment of processing. How to change this?

Answer 1

Since you are calling parse_excel_rfi_sheet from ExcelFileUploadView whenever the exception {"general_errors": ["Error during excel file parsing. Unknown module cell"]} is raised from parse_excel_rfi_sheet function try block from ExcelFileUploadView fails and comes to except and raises the exception {"general_errors": ["Error during file upload"]}.

You can verify this by printing the exception raised by the ExcelFileUploadView function. Chane the try block to the following:

try:
    file = default_storage.save(filename, f)
    r = parse_excel_rfi_sheet(file)
    status = 200
except Exception as e:
    print("Exception raised ", e)
    raise Exception({"general_errors": ["Error during file upload"]})

Answer 2

Your problem comes from catching absolutely all exceptions, first in parse_excel_rfi_sheet, then once again in your put method. Both bare except clause (except: whatever_code_here) and large try blocks are antipatterns - you only want to catch the exact exceptions you're expecting at a given point (using except (SomeExceptionType, AnotherExceptionType, ...) as e:, and have as few code as possible in your try blocks so you are confident you know where the exception comes from.

The only exception (no pun intended) to this rule is the case of "catch all" handlers at a higher level, that are use to catch unexpected errors, log them (so you have a trace of what happened), and present a friendly error message to the user - but even then, you don't want a bare except clause but a except Exception as e.

TL;DR: never assume anything about which exception was raised, where and why, and never pass exceptions silently (at least log them - and check your logs).

Answer 3

raise Exception(...) generates a new Exception instance and raises that one. This means, the try ... except in put effectively throws away the exception it caught and replaces it with a new one with message "Error during file upload", which is why you always see the same message.

A clean way to handle this would be to define a custom subclass of Exception (e.g., InvalidFormatException) and raise that one in parse_excel_rfi_sheet, having two different except cases in put:

class InvalidFormatException(Exception):
    pass

[...]

def parse_excel_rfi_sheet(file):
    workbook = load_workbook(filename=file)
    sheet = workbook["RFI"]
    curent_module_coordinate = []
    try:
        ....
        curent_module_coordinate.append(sheet['E688'].value)  
        curent_module_coordinate.append(sheet['E950'].value)  
        if check_exel_rfi_template_structure(structure=curent_module_coordinate):
            file_status = True
        else:
            file_status = False
    except:
        raise InvalidFormatException({"general_errors": ["Error during excel file parsing. Unknown module cell"]})

Your put then becomes:

    def put(self, request, format=None):
        if 'file' not in request.data:
            raise ParseError("Empty content")
        f = request.data['file']
        filename = f.name
        if filename.endswith('.xlsx'):
            try:
                file = default_storage.save(filename, f)
                r = parse_excel_rfi_sheet(file)
                status = 200
            except InvalidFormatException:
                raise # pass on the exception
            except:
                raise Exception({"general_errors": ["Error during file upload"]})
            finally:
                default_storage.delete(file)
        else:
            status = 406
            r = {"general_errors": ["Please upload only xlsx files"]}
        return Response(r, status=status)

Warning: As pointed out in the comments to this answer, note that -although not directly inquired- the OP's code should be further modified to remove the bare except: clause, as this is probably not the expected behaviour.