Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Feature]: Postgres error message improvements on connectivity #19723

Closed
1 task done
AnandShahs123 opened this issue Jan 11, 2023 · 3 comments · Fixed by #35167
Closed
1 task done

[Feature]: Postgres error message improvements on connectivity #19723

AnandShahs123 opened this issue Jan 11, 2023 · 3 comments · Fixed by #35167
Assignees
@Nikhil-Nandagopal @sribalajig @riteshkew @rohan-arthur
Labels
Enhancement New feature or request Integrations Product Issues related to a specific integration Postgres Postgres related issues Query & JS Pod Issues related to the query & JS Pod

Comments

@AnandShahs123
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Summary

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

  2. If host address or port is wrong, there is no reference of that in the error message.

3.There is no error message when the port number is left empty.

error1

Screenshot 2023-01-11 at 1 42 11 PM

Why should this be worked on?

  1. The users should be able to see the error message wherever required.

  2. The users should be able to see the right error message
@AnandShahs123 AnandShahs123 added Enhancement New feature or request Low An issue that is neither critical nor breaks a user flow Postgres Postgres related issues labels Jan 11, 2023
@github-actions github-actions bot added the Integrations Product Issues related to a specific integration label Jan 11, 2023
@Nikhil-Nandagopal
Copy link
Contributor

@rohan-arthur seems like simple datasource form validation

@sribalajig sribalajig added Data Platform Pod Issues related to the underlying data platform Datasources Issues related to configuring datasource on appsmith Error Handling High This issue blocks a user from building or impacts a lot of users and removed Low An issue that is neither critical nor breaks a user flow Integrations Product Issues related to a specific integration labels Jan 23, 2023
@github-actions github-actions bot added Integrations Product Issues related to a specific integration Query & JS Pod Issues related to the query & JS Pod labels Jan 23, 2023
@sribalajig sribalajig removed Integrations Product Issues related to a specific integration Postgres Postgres related issues labels Jan 26, 2023
@github-actions github-actions bot added the Integrations Product Issues related to a specific integration label Jan 26, 2023
@sribalajig sribalajig removed Integrations Product Issues related to a specific integration Datasources Issues related to configuring datasource on appsmith labels Jan 26, 2023
@nidhi-nair nidhi-nair added Postgres Postgres related issues and removed Error Handling Data Platform Pod Issues related to the underlying data platform labels Jul 22, 2023
@github-actions github-actions bot added Integrations Product Issues related to a specific integration and removed Query & JS Pod Issues related to the query & JS Pod labels Jul 22, 2023
@daaliachhak17
Copy link

created a jira ticket-https://zemoso.atlassian.net/browse/TNOSB-349

@AnnaHariprasad5123 AnnaHariprasad5123 mentioned this issue Jul 25, 2024
Merged
@AnnaHariprasad5123
Copy link
Contributor

Hi @sribalajig,

I have raised a PR. Could you please review it?

Thank you.

@Nikhil-Nandagopal Nikhil-Nandagopal removed the High This issue blocks a user from building or impacts a lot of users label Jul 29, 2024
@github-actions github-actions bot added the Query & JS Pod Issues related to the query & JS Pod label Jul 29, 2024
NilanshBansal added a commit that referenced this issue Sep 6, 2024
@NilanshBansal
Merge branch 'Issue-#19723/feat-postgres-conn-error-msg-improvements'…
09ed8b9 
… of github.com:AnnaHariprasad5123/appsmith into chore/external-contri-ce-35167
NilanshBansal added a commit that referenced this issue Sep 9, 2024
@NilanshBansal
Merge branch 'Issue-#19723/feat-postgres-conn-error-msg-improvements'…
3c97f02 
… of github.com:AnnaHariprasad5123/appsmith into chore/external-contri-ce-35167
@nerbos nerbos closed this as completed in 1cf452f Sep 9, 2024
Shivam-z pushed a commit to Shivam-z/appsmith that referenced this issue Sep 26, 2024
@AnnaHariprasad5123 @Shivam-z
feat: Improved-error-messages-for-postgres-connection (appsmithorg#35167
7d211be 
)

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

Fixes appsmithorg#19723 

**What’s in this PR:**

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

2. In PostgresErrorMessages

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

3. In PostgresPluginTest

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

**Screenshots :**
1. 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

![image](https://github.com/user-attachments/assets/a54ff455-7297-4356-bf81-656c1130977a)
2. If host address or port is wrong, there is no reference of that in
the error message. - Fixed

![image](https://github.com/user-attachments/assets/c48dabdc-9957-459a-9dde-d4b73960b9e2)
3. There is no error message when the port number is left empty. - Fixed

![image](https://github.com/user-attachments/assets/2c7967c8-b4e1-4bb2-a8e5-14e8022bca29)

**Test cases :**

![image](https://github.com/user-attachments/assets/329a7453-c1ba-4a85-b807-2cdf4569db3e)

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

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

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Nikhil-Nandagopal Nikhil-Nandagopal

@sribalajig sribalajig

@riteshkew riteshkew

@rohan-arthur rohan-arthur

Labels
Enhancement New feature or request Integrations Product Issues related to a specific integration Postgres Postgres related issues Query & JS Pod Issues related to the query & JS Pod
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Improved-error-messages-for-postgres-connection AnnaHariprasad5123/appsmith
8 participants
@Nikhil-Nandagopal @nidhi-nair @sribalajig @riteshkew @rohan-arthur @AnandShahs123 @AnnaHariprasad5123 @daaliachhak17