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.

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

remove the exception message, and use report #69

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

atmonshi
Copy link
Contributor

@atmonshi atmonshi commented Oct 9, 2024

hey Chris, so I got this error as you can see the message contain kinda sensitive info about the database when the expiation AbortedLoginException thrown.
so I replaced that with Login aborted instead of $e->getMessage()

the other thing that I didn't get an exception alert on sentry (or any other error reporting tools) coz the exception been handled by (try/catch), so I added report($e);.

@chrisreedio
Copy link
Owner

Thanks for your PR @atmonshi !

Perhaps we can tweak what data is in the exception message?

The thought was that if the developer needed to handle any extra cases in which the login would be aborted they could use the exception message to have it shown in the error message.

Sorry if I disappear out of the blue, my wife is due to go into labor any day so I may be out of pocket at a moments notice.

@atmonshi
Copy link
Contributor Author

hey Chris, no worries.
I will take another look later tonight.

and congrats, that's so exciting! wishing you both a smooth and safe delivery.

@chrisreedio
Copy link
Owner

hey Chris, no worries. I will take another look later tonight.

and congrats, that's so exciting! wishing you both a smooth and safe delivery.

@atmonshi We had a great delivery but time slips away faster now than ever.

Did you ever take another look at this? Absolutely no rush just saw the PR and reminded me to check in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants