wary commented on code in PR #309:
URL: 
https://github.com/apache/doris-spark-connector/pull/309#discussion_r2048183473


##########
spark-doris-connector/spark-doris-connector-base/src/main/java/org/apache/doris/spark/client/DorisFrontendClient.java:
##########
@@ -114,7 +115,13 @@ private List<Frontend> initFrontends(DorisConfig config) 
throws Exception {
                                         + ", reason: " + 
response.getStatusLine().getReasonPhrase());
                             }
                             String entity = 
EntityUtils.toString(response.getEntity());
-                            dataNode = extractEntity(entity, "data");
+                                                       LOG.info("get response: 
{} with fe request url: {}", entity,  feReqURL);
+                            JsonNode respNode = MAPPER.readTree(entity);
+                            String code = respNode.get("code").asText();
+                            if(!"0".equalsIgnoreCase(code)) {
+                                throw new RuntimeException("fetch fe request 
with invalid code, response: " + entity);
+                            }
+                            dataNode = respNode.get("data");

Review Comment:
   
   > Maybe I didn't express it clearly.
   > 
   >     1. Since the "extractEntity" method is called in many places, the 
calls in other places may also have this problem, and other places may also 
need to be dealt with?
   > 
   Yes, Doris Spark Connector has similar issues in many other cases, where it 
only checks the HTTP code but doesn't verify the status or code in the 
response, just like this commit operation in previous version.
   >     2. Does this exception necessarily need to be thrown? If it doesn't 
need to be thrown, perhaps just printing a log entry will be enough?
   No matter whether the exception is thrown or not, the task will fail due to 
this error. If we don't throw this exception, the error message won't be 
accurate enough, causing many misunderstandings during troubleshooting. I think 
this follows the same logic as the HTTP status code check above—it should be 
thrown.
   
   



-- 
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.

To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org

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


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

Reply via email to