Skip to content

remove the exception message, and use report#69

Open
atmonshi wants to merge 1 commit intochrisreedio:3.xfrom
atmonshi:report-errors
Open

remove the exception message, and use report#69
atmonshi wants to merge 1 commit intochrisreedio:3.xfrom
atmonshi:report-errors

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