You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by denalex <gi...@git.apache.org> on 2018/02/05 20:16:18 UTC

[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

Github user denalex commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1334#discussion_r166099466
  
    --- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java ---
    @@ -143,36 +142,38 @@ private static synchronized Response synchronizedWriteResponse(Bridge bridge,
     
         private static Response writeResponse(Bridge bridge,
                                               String path,
    -                                          InputStream inputStream) throws Exception {
    -
    -        String returnMsg;
    -
    +                                          InputStream inputStream)
    +            throws Exception {
             // Open the output file
             bridge.beginIteration();
    -
             long totalWritten = 0;
    +        Exception ex = null;
     
             // dataStream will close automatically in the end of the try.
             // inputStream is closed by dataStream.close().
             try (DataInputStream dataStream = new DataInputStream(inputStream)) {
                 while (bridge.setNext(dataStream)) {
                     ++totalWritten;
                 }
    -        } catch (ClientAbortException e) {
    -            LOG.debug("Remote connection closed by HAWQ", e);
    -        } catch (Exception ex) {
    -            LOG.debug("totalWritten so far " + totalWritten + " to " + path);
    -            throw ex;
    +        } catch (ClientAbortException cae) {
    +            LOG.error("Remote connection closed by HAWQ", cae);
    +        } catch (Exception e) {
    +            LOG.error("Exception: totalWritten so far " + totalWritten + " to " + path, e);
    +            ex = e;
             } finally {
                 try {
                     bridge.endIteration();
                 } catch (Exception e) {
    -                // ignore ... any significant errors should already have been handled
    +                if (ex == null)
    +                    ex = e;
    --- End diff --
    
    Another way would be to preserve throwing the exception inside original catch block (line 162), then here you would say
    ```
    if (ex == null) 
         throw e;
     else 
         throw ex;
    ``` 
    and you would not need the block below (lines 170-172) as the original will still be thrown if endIterations() completes without an error.


---