Servlet file uploader

I think this code looks ugly, especially with the multiple try-catch blocks, but I don't know how to rewrite it:

public String handleFileUpload(@RequestParam String url,@RequestParam String fileName, RedirectAttributes redirectAttributes, HttpServletResponse response) {
        InputStream inputStream;
        try {
             inputStream =  new URL(url).openStream();
        } catch (IOException e) {
            LOG.error("Cannot open stream by url=[{}]", url);
            try {
                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Cannot open stream by url=" + url);
            } catch (IOException e1) {
                LOG.error("Cannot send error");
            }
            return null;
        }
        try {
            File file = File.createTempFile("tmp", ".txt", new File(System.getProperty("user.dir")));
            byte[] binary = IOUtils.toByteArray(inputStream);
            FileUtils.writeByteArrayToFile(file, binary);
            UploadedMultipartFile multipartFile = new UploadedMultipartFile(file, file.length(), "jpg",
                    "formParameter", fileName);
            MultipartFileWrapper multipartFileWrapper = new MultipartFileWrapper();
            multipartFileWrapper.setMultipartFile(multipartFile);
            redirectAttributes.addFlashAttribute(multipartFileWrapper);
        } catch (IOException e) {
            LOG.error("Cannot save file [{}] from [{}]",fileName, url);
            try {
                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Cannot save file " + fileName);
            } catch (IOException e1) {
                LOG.error("Cannot send error");
            }
            return null;
        }
        return "redirect:/member/uploadImage";
    }

Two try blocks in a method is a smell IMO: your method is trying to do two things, which implies it's not doing one and only one thing.

Let's see:

• We open a stream from a given url and if all goes well we have an inputStream to work with.

• Or things go bad, and we log an error, and then try to send an error response before we return null - if returning the error response failed, we log and still return null.

• We use the stream to create a new file, and return a hard-coded string on success, or null on failure.

That alone looks like a bit of reusable functionality that should be extracted into its own method: one whose responsibility would be to send an error response:

void sendErrorResponse(HttpServletResponse response, string message) {
    try {
        response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, message);
    } catch(IOException e) {
        LOG.error("An error occurred while sending error response: " + e.toString());
    }
}

Next, you need a function that takes a url and gives you an inputStream.

InputStream openStreamFromUrl(string url, HttpServletResponse response) {
    InputStream result;
    try {
        result = new URL(url).openStream();
    }
    catch(IOException e) {
        string message = "Cannot open stream for url=" + url;
        LOG.error(message);
        sendErrorResponse(response, message);
    }
    return result;
}

So calling this openStreamFromUrl function either returns a valid and opened InputStream, or a null reference. This means we can now do this:

public String handleFileUpload(@RequestParam String url,@RequestParam String fileName, RedirectAttributes redirectAttributes, HttpServletResponse response) {
    InputStream inputStream = openStreamFromUrl(url, response);
    if (inputStream == null) {
        return null;
    }

    // handle file upload
}

StackExchange Code Review Q#101800, answer score: 11