appsmith feat: Improved-error-messages-for-postgres-connection

Hi @appsmithorg/contributor-support, @rohan-arthur, @nidhi-nair

Fixes #19723

What’s in this PR:

In Postgres Plugin:

Added two imports: PSQLException and PSQLState to identify the type of exception and state of error.
Added a condition to validate the empty port field.
Removed the default port when the port is empty.
Removed “Failed to initialize pool:” term in the error message.

In PostgresErrorMessages

Added two constant value for Missing port and invalid host and port.

In PostgresPluginTest

Added a test case to validate the missing port error message.

Screenshots :

If any of the field like host address, port, DB name etc is wrong - There is a term in the error message along with the error message in the toast "Failed to initialize pool: ". This is not required. - Fixed
If host address or port is wrong, there is no reference of that in the error message. - Fixed
There is no error message when the port number is left empty. - Fixed

Test cases :

Why I didn’t provide test cases for invalid host and port :

Those errors are triggered by the createConnectionPool method, which is private, so we can’t access it in the test file.

Summary by CodeRabbit

New Features
- Improved validation for PostgreSQL datasource configurations, ensuring that a port is specified.
- Enhanced error messaging for connection issues related to missing or invalid port and hostname.
Bug Fixes
- Refined error handling during connection pool initialization for clearer feedback on specific connection issues.
Tests
- Added a test to validate behavior when the datasource configuration has an empty port, improving coverage for validation logic.

Jul 25 '24 05:07 AnnaHariprasad5123

Walkthrough

The updates enhance the PostgreSQL plugin by improving validation and error messaging for datasource configurations. Key modifications include checks for missing ports, refined JDBC URL construction, and enhanced error handling during connection pool initialization. These changes ensure clearer feedback for users, allowing for better troubleshooting of connectivity issues.

Changes

File	Change Summary
`.../PostgresPlugin.java`	Added validation for empty ports, refined connection pool logic, and improved error handling for connection exceptions.
`.../PostgresErrorMessages.java`	Introduced new error message constants for missing ports and invalid host/port configurations to enhance user guidance.
`.../PostgresPluginTest.java`	Added a new test to validate behavior when the datasource configuration has an empty port, improving test coverage.

Assessment against linked issues

Objective	Addressed	Explanation
Error message improvements for wrong host/port and empty port (19723)	✅
Clearer user feedback for connection issues (19723)	✅
Removal of unnecessary error message terms (19723)	❌	The term "Failed to initialize pool:" is still present in error messages.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Jul 25 '24 05:07 coderabbitai[bot]

Hi @nidhi-nair, Could you review this pr or assign anyone to review this pr.

Jul 31 '24 06:07 AnnaHariprasad5123

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

Aug 07 '24 16:08 github-actions[bot]

Hi @rohan-arthur, @nidhi-nair Could you please review this pr.

Aug 08 '24 01:08 AnnaHariprasad5123

Hi @nidhi-nair, Can you assign anyone to review this pr.

Aug 20 '24 03:08 AnnaHariprasad5123

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

Aug 27 '24 16:08 github-actions[bot]

Commenting to keep the PR alive

Aug 28 '24 04:08 NilanshBansal

@AnnaHariprasad5123 can you resolve merge conflicts here

Sep 02 '24 06:09 NilanshBansal

Hi @NilanshBansal, Now we can't get "Missing Port" error. Because it will take default port value. If I remove default port there, users DB will not work which we face in my previous pr.

Sep 02 '24 11:09 AnnaHariprasad5123

Hi @NilanshBansal, Now we can't get "Missing Port" error. Because it will take default port value. If I remove default port there, users DB will not work which we face in my previous pr.

Yes, that is fine @AnnaHariprasad5123.

Sep 02 '24 11:09 NilanshBansal

Hi @NilanshBansal

Good morning, I removed the code related to Missing Port error that fixed the cypress tests. Remaining cypress tests are failed due to docker.internal url.

Screenshots :

cypress/e2e/Regression/ClientSide/MobileResponsiveTests/ConversionFlow_Generated_App_spec.ts
cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts
cypress/e2e/Regression/ClientSide/OtherUIFeatures/EntityBottomBar_spec.ts
cypress/e2e/Smoke/GenerateCRUD/Generate_Crud_New_Page_spec.ts

Sep 03 '24 02:09 AnnaHariprasad5123

Hi @NilanshBansal, Could you check this pr.

Sep 06 '24 01:09 AnnaHariprasad5123

/build-deploy-preview skip-tests=true

Sep 06 '24 05:09 NilanshBansal

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10733008767. Workflow: On demand build Docker image and deploy preview. skip-tests: true. env: ``. PR: 35167. recreate: .

Sep 06 '24 05:09 github-actions[bot]

Deploy-Preview-URL: https://ce-35167.dp.appsmith.com

Sep 06 '24 05:09 github-actions[bot]

@AnnaHariprasad5123 can you resolve merge conflicts by taking the latest pull. Also, there are a number of indentation changes in your last commit can you please check how these have appeared

Sep 06 '24 06:09 NilanshBansal

@AnnaHariprasad5123 Please check the error messaging it has a full stop and a comma together. Comma should not be present SCR-20240906-h4g

Sep 06 '24 06:09 NilanshBansal

Hi @NilanshBansal Good morning, Could you check once now.

Sep 09 '24 05:09 AnnaHariprasad5123

@AnnaHariprasad5123 can you also check why there are a number of indentation changes in your last commit. This makes it very difficult to review. Can you please remove the unnecessary indentation changes and run mvn spotless:apply and then commit.

Sep 09 '24 07:09 NilanshBansal

@AnnaHariprasad5123 can you also check why there are a number of indentation changes in your last commit. This makes it very difficult to review. Can you please remove the unnecessary indentation changes and run mvn spotless:apply and then commit.

When I save a file, the code automatically changes its formatting. This makes the lines look indented, even if I delete the extra spaces after saving it got formatted. When I run the mvn spotless:apply check command, it fails because the code doesn't match the desired formatting rules.

Sep 09 '24 07:09 AnnaHariprasad5123

@AnnaHariprasad5123 can you check your IDE formatter rules. These are the config from my IDE

Sep 09 '24 07:09 NilanshBansal

@NilanshBansal , Same config from my side already.

Sep 09 '24 07:09 AnnaHariprasad5123

Hi @NilanshBansal, Done mvn spotless:apply. Postgres plugin resets indentations. Could you check once now.

Sep 09 '24 07:09 AnnaHariprasad5123

/build-deploy-preview skip-tests=true recreate=true

Sep 09 '24 07:09 NilanshBansal

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10768967470. Workflow: On demand build Docker image and deploy preview. skip-tests: true. env: ``. PR: 35167. recreate: true.

Sep 09 '24 07:09 github-actions[bot]

Deploy-Preview-URL: https://ce-35167.dp.appsmith.com

Sep 09 '24 08:09 github-actions[bot]

@AnnaHariprasad5123 can you please also add a unit test for this scenario. You can take reference from the coderabbit suggestion here https://github.com/appsmithorg/appsmith/pull/36053#discussion_r1749803876

Sep 09 '24 08:09 NilanshBansal

Hi @NilanshBansal, createConnectionPool method is private method. Is it good to write test case for private methods?

Sep 09 '24 08:09 AnnaHariprasad5123

Hi @NilanshBansal, createConnectionPool method is private method. Is it good to write test case for private methods?

@AnnaHariprasad5123 thanks for highlighting we can skip it in this scenario.

Sep 09 '24 10:09 NilanshBansal