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