2
\$\begingroup\$

I am learning exception handling in java. Every time I am writing a function, I wonder if I should put the entire function code in a try catch block.

What the function does:

In the below function, i am checking for file upload failures that could be caused by multiple issues.e.g. file too big, not a supported format and so on.

In the function, i add try/catch just to a small part because it sometime throws some network exceptions e.g. connection timeout, socket timeout, network unreachable and so on.

What improvements i am looking for:

I want the network exceptions to be handled by the already present try catch and add a new try/catch that should take care of exceptions that may arise from other parts of function.

public void checkFile(FacesContext ctx, UIComponent comp, Object value) throws IOException {  
        file = (Part) value;
        if (file != null) {
            String originalFileName = FileUtil.getFileName(file);
            String fileContentType = file.getContentType();

            if (file.getSize() > MAX_UPLOAD_SIZE) {
                msgs.add(new FacesMessage(FacesMessage.SEVERITY_ERROR, "File too big.", "The given file was too big." +
                        " Maximum size allowed is: " + MAX_UPLOAD_SIZE / 1024 / 1024 + " MB"));
                throw new ValidatorException(msgs);
            }

            if (fileContentType.matches("application/x-msdownload|application/x-ms-installer|application/x-elf|application/x-sh|text/x-perl|text/x-python")) {
                msgs.add(new FacesMessage(FacesMessage.SEVERITY_ERROR, "Not a permissible file type", "The uploaded file is " +
                        "an executable file."));
                throw new ValidatorException(msgs);
            }
            try {
                if (FileUtil.checkVirusClamAV(ProjectConfig.get().getFileUploader(), file.getInputStream())) {
                    msgs.add(new FacesMessage(FacesMessage.SEVERITY_ERROR, "File possibly infected",
                            "The uploaded file triggered a virus warning and therefore has been rejected."));
                    throw new ValidatorException(msgs);
                }
            } catch(Exception e) {
                msgs.add(new FacesMessage(FacesMessage.SEVERITY_ERROR, "File possibly infected",
                        "The uploaded file triggered a virus warning and therefore has been rejected."));

                generateAntiVirusExceptionMessages(e.getMessage());
                throw new ValidatorException(msgs);
            }

            if ((originalFileName != null) && (!FilenameUtils.getExtension(originalFileName).equals("xml"))) {
                //Generate validation message for non-xml files
                generateValidationMessages(validationMessageType.nonXmlSuccess);
            }

            if (originalFileName != null && FilenameUtils.getExtension(originalFileName).equals("xml")) {
                //Do validation only for xml files
                validateXmlFiles();
            }
        }
    }
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! What task does this code accomplish? Please tell us, and also make that the title of the question via edit. Maybe you missed the placeholder on the title element: "State the task that your code accomplishes. Make your title distinctive.". Also from How to Ask: "State what your code does in your title, not your main concerns about it.". \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 11 '18 at 15:25

Browse other questions tagged or ask your own question.