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
}