CaoManhDat commented on a change in pull request #1470:
URL: https://github.com/apache/lucene-solr/pull/1470#discussion_r418399765



##########
File path: 
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
##########
@@ -359,65 +366,95 @@ public void send(OutStream outStream, SolrRequest req, 
String collection) throws
     outStream.flush();
   }
 
-  public NamedList<Object> request(SolrRequest solrRequest,
-                                      String collection,
-                                      OnComplete onComplete) throws 
IOException, SolrServerException {
-    Request req = makeRequest(solrRequest, collection);
+  private static final Exception CANCELLED_EXCEPTION = new Exception();
+
+  public Cancellable asyncRequest(SolrRequest solrRequest, String collection, 
OnComplete onComplete) {
+    Request req;
+    try {
+      req = makeRequest(solrRequest, collection);
+    } catch (SolrServerException | IOException e) {
+      onComplete.onFailure(e);
+      return () -> {};
+    }
     final ResponseParser parser = solrRequest.getResponseParser() == null
         ? this.parser: solrRequest.getResponseParser();
-
-    if (onComplete != null) {
-      // This async call only suitable for indexing since the response size is 
limited by 5MB
-      req.onRequestQueued(asyncTracker.queuedListener)
-          .onComplete(asyncTracker.completeListener).send(new 
BufferingResponseListener(5 * 1024 * 1024) {
-
-        @Override
-        public void onComplete(Result result) {
-          if (result.isFailed()) {
-            onComplete.onFailure(result.getFailure());
-            return;
+    req.onRequestQueued(asyncTracker.queuedListener)
+        .onComplete(asyncTracker.completeListener)
+        .send(new InputStreamResponseListener() {
+          @Override
+          public void onHeaders(Response response) {
+            super.onHeaders(response);
+            InputStreamResponseListener listener = this;
+            executor.execute(() -> {
+              InputStream is = listener.getInputStream();
+              assert ObjectReleaseTracker.track(is);
+              try {
+                NamedList<Object> body = processErrorsAndResponse(response, 
parser, is, getEncoding(response), isV2ApiRequest(solrRequest));
+                onComplete.onSuccess(body);
+              } catch (RemoteSolrException e) {
+                if (SolrException.getRootCause(e) != CANCELLED_EXCEPTION) {
+                  onComplete.onFailure(e);
+                }
+              } catch (SolrServerException e) {
+                onComplete.onFailure(e);
+              }
+            });
           }
 
-          NamedList<Object> rsp;
-          try {
-            InputStream is = getContentAsInputStream();
-            assert ObjectReleaseTracker.track(is);
-            rsp = processErrorsAndResponse(result.getResponse(),
-                parser, is, getEncoding(), isV2ApiRequest(solrRequest));
-            onComplete.onSuccess(rsp);
-          } catch (Exception e) {
-            onComplete.onFailure(e);
+          @Override
+          public void onFailure(Response response, Throwable failure) {
+            super.onFailure(response, failure);
+            if (failure != CANCELLED_EXCEPTION) {
+              onComplete.onFailure(createException(req, failure));
+            }
           }
-        }
-      });
-      return null;
-    } else {
-      try {
-        InputStreamResponseListener listener = new 
InputStreamResponseListener();
-        req.send(listener);
-        Response response = listener.get(idleTimeout, TimeUnit.MILLISECONDS);
-        InputStream is = listener.getInputStream();
-        assert ObjectReleaseTracker.track(is);
-        return processErrorsAndResponse(response, parser, is, 
getEncoding(response), isV2ApiRequest(solrRequest));
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-        throw new RuntimeException(e);
-      } catch (TimeoutException e) {
-        throw new SolrServerException(
-            "Timeout occured while waiting response from server at: " + 
req.getURI(), e);
-      } catch (ExecutionException e) {
-        Throwable cause = e.getCause();
-        if (cause instanceof ConnectException) {
-          throw new SolrServerException("Server refused connection at: " + 
req.getURI(), cause);
-        }
-        if (cause instanceof SolrServerException) {
-          throw (SolrServerException) cause;
-        } else if (cause instanceof IOException) {
-          throw new SolrServerException(
-              "IOException occured when talking to server at: " + 
getBaseURL(), cause);
-        }
-        throw new SolrServerException(cause.getMessage(), cause);
+        });
+    return () -> req.abort(CANCELLED_EXCEPTION);
+  }
+
+  @Override
+  public NamedList<Object> request(SolrRequest solrRequest, String collection) 
throws SolrServerException, IOException {
+    Request req = makeRequest(solrRequest, collection);
+    final ResponseParser parser = solrRequest.getResponseParser() == null
+        ? this.parser: solrRequest.getResponseParser();
+
+    try {
+      InputStreamResponseListener listener = new InputStreamResponseListener();
+      req.send(listener);
+      Response response = listener.get(idleTimeout, TimeUnit.MILLISECONDS);
+      InputStream is = listener.getInputStream();
+      assert ObjectReleaseTracker.track(is);
+      return processErrorsAndResponse(response, parser, is, 
getEncoding(response), isV2ApiRequest(solrRequest));
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new RuntimeException(e);
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Throwable e) {
+      throw createException(req, e);

Review comment:
       Right, I think it is worth to seperate the exception handling logic of 
asyncReq() and req().




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to