Fix druid throwing "Invalid type marker byte 0x3c for expected value token"...

Fix druid throwing "Invalid type marker byte 0x3c for expected value token" errors if limit is 1048575  (#15452)

* Better error logging for druid

* Apparently Druid queries don't like the number 1058575

* Test fix 

modules/drivers/druid/src/metabase/driver/druid/client.clj

@@ -24,17 +24,32 @@
   [request-fn url & {:as options}]
   {:pre [(fn? request-fn) (string? url)]}
   ;; this is the way the `Content-Type` header is formatted in requests made by the Druid web interface
-  (let [options               (cond-> (merge {:content-type "application/json;charset=UTF-8"} options)
-                                (:body options) (update :body json/generate-string))
-        {:keys [status body]} (request-fn url options)]
-    (when (not= status 200)
-      (throw (ex-info (tru "Druid request error [{0}]: {1}" status (pr-str body))
-                      {:type qp.error-type/db})))
+  (let [options (cond-> (merge {:content-type "application/json;charset=UTF-8"} options)
+                  (:body options) (update :body json/generate-string))]
     (try
-      (json/parse-string body keyword)
-      (catch Throwable _
-        (throw (ex-info (tru "Failed to parse Druid response body: {0}" (pr-str body))
-                        {:type qp.error-type/db}))))))
+      (let [{:keys [status body]} (request-fn url options)]
+        (when (not= status 200)
+          (throw (ex-info (tru "Druid request error [{0}]: {1}" status (pr-str body))
+                          {:type qp.error-type/db})))
+        (try
+          (json/parse-string body keyword)
+          (catch Throwable e
+            (throw (ex-info (tru "Failed to parse Druid response body: {0}" (pr-str body))
+                            {:type qp.error-type/db}
+                            e)))))
+      (catch Throwable e
+        (let [response (u/ignore-exceptions
+                         (when-let [body (:body (:object (ex-data e)))]
+                           (json/parse-string body keyword)))]
+          (throw (ex-info (or (:errorMessage response)
+                              (.getMessage e))
+                          (merge
+                           {:type            qp.error-type/db
+                            :request-url     url
+                            :request-options options}
+                           (when response
+                             {:response response}))
+                          e)))))))
 
 (def ^{:arglists '([url & {:as options}])} GET    "Execute a GET request."    (partial do-request http/get))
 (def ^{:arglists '([url & {:as options}])} POST   "Execute a POST request."   (partial do-request http/post))
@@ -51,20 +66,13 @@
       (catch InterruptedException e
         (throw e))
       (catch Throwable e
-        ;; try to extract the error
-        (let [message (or (u/ignore-exceptions
-                           (when-let [body (json/parse-string (:body (:object (ex-data e))) keyword)]
-                             (str (:error body) "\n"
-                                  (:errorMessage body) "\n"
-                                  "Error class:" (:errorClass body))))
-                          (.getMessage e))]
-          (log/error (u/format-color 'red (trs "Error running query:")
-                                     "\n" message))
-          ;; Re-throw a new exception with `message` set to the extracted message
-          (throw (ex-info message
-                          {:type  qp.error-type/driver
+        (let [e' (ex-info (.getMessage e)
+                          {:type  qp.error-type/db
                            :query query}
-                          e)))))))
+                          e)]
+          (log/error e' (trs "Error running query"))
+          ;; Re-throw a new exception with `message` set to the extracted message
+          (throw e'))))))
 
 (defn- cancel-query-with-id! [details query-id]
   (if-not query-id

modules/drivers/druid/src/metabase/driver/druid/execute.clj

@@ -155,7 +155,13 @@
                      query)
         query-type (or query-type
                        (keyword (namespace ::druid.qp/query) (name (:queryType query))))
-        results    (execute* details query)
+        results    (try
+                     (execute* details query)
+                     (catch Throwable e
+                       (throw (ex-info (tru "Error executing query")
+                                       {:type  qp.error-type/db
+                                        :query query}
+                                       e))))
         result     (try (post-process query-type projections
                                       {:timezone   (resolve-timezone mbql-query)
                                        :middleware middleware}

modules/drivers/druid/src/metabase/driver/druid/query_processor.clj

@@ -1085,11 +1085,18 @@
   {:arglists '([query-type original-query druid-query])}
   query-type-dispatch-fn)
 
+(defn- adjust-limit
+  "No joke, Druid queries do not work if limit is `1048575`, but they work if limit is `1048576`. They fail with an
+  'Invalid type marker byte 0x3c for expected value token` error. So if we see the updated `absolute-max-results` from
+  #15414, adjust it back to the old known working value. had to work around."
+  [limit]
+  (cond-> limit
+    (= limit i/absolute-max-results) inc))
+
 (defmethod handle-limit ::scan
   [_ {limit :limit} druid-query]
-  (if-not limit
-    druid-query
-    (assoc-in druid-query [:query :limit] limit)))
+  (cond-> druid-query
+    limit (assoc-in [:query :limit] (adjust-limit limit))))
 
 (defmethod handle-limit ::timeseries
   [_ {limit :limit} druid-query]
@@ -1101,18 +1108,14 @@
 
 (defmethod handle-limit ::topN
   [_ {limit :limit} druid-query]
-  (if-not limit
-    druid-query
-    (assoc-in druid-query [:query :threshold] limit)))
+  (cond-> druid-query
+    limit (assoc-in [:query :threshold] (adjust-limit limit))))
 
 (defmethod handle-limit ::groupBy
   [_ {limit :limit} druid-query]
-  (if-not limit
-    (-> druid-query
-        (assoc-in [:query :limitSpec :type]  :default))
-    (-> druid-query
-        (assoc-in [:query :limitSpec :type]  :default)
-        (assoc-in [:query :limitSpec :limit] limit))))
+  (cond-> druid-query
+    true  (assoc-in [:query :limitSpec :type]  :default)
+    limit (assoc-in [:query :limitSpec :limit] (adjust-limit limit))))
 
 
 ;;; -------------------------------------------------- handle-page ---------------------------------------------------