Re: [PR] chore: make DB syntax errors 400 [superset]

Thu, 29 May 2025 11:31:01 -0700 


korbit-ai[bot] commented on code in PR #33619:
URL: https://github.com/apache/superset/pull/33619#discussion_r2114511766


##########
superset/commands/database/validate.py:
##########
@@ -125,7 +125,7 @@ def run(self) -> None:
                     "database": url.database,
                 }
                 errors = database.db_engine_spec.extract_errors(ex, context)
-                raise DatabaseTestConnectionFailedError(errors) from ex
+                raise DatabaseTestConnectionFailedError(errors, status=400) 
from ex

Review Comment:
   ### Incorrect Error Status Code Classification <sub>![category Error 
Handling](https://img.shields.io/badge/Error%20Handling-ea580c)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The code universally applies a 400 status code to all database connection 
errors, but not all database connection errors are client-side errors (400).
   
   
   ###### Why this matters
   Some database connection errors could be server-side issues (e.g., database 
server down, network issues) which should return 500 status codes. This could 
mislead clients about the nature of the error and cause incorrect error 
handling.
   
   ###### Suggested change ∙ *Feature Preview*
   Add logic to differentiate between client-side and server-side errors. For 
example:
   ```python
   errors = database.db_engine_spec.extract_errors(ex, context)
   status = 400 if database.db_engine_spec.is_syntax_error(ex) else 500
   raise DatabaseTestConnectionFailedError(errors, status=status) from ex
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/561e1991-ae97-4184-8fe6-18a01b161910/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/561e1991-ae97-4184-8fe6-18a01b161910?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/561e1991-ae97-4184-8fe6-18a01b161910?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/561e1991-ae97-4184-8fe6-18a01b161910?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/561e1991-ae97-4184-8fe6-18a01b161910)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:00fe8e1c-2695-4d52-bf0c-b51278fcee0e -->
   
   
   [](00fe8e1c-2695-4d52-bf0c-b51278fcee0e)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to