Stop catching exceptions and destroying them

[oliverde8]

Summary (*)

Magento catches very often \Exception and then does nothing with it. No logs nothing.

This means when such an error happens everyone thinks all is well, but in reality, hell might be breaking loose.

Examples (*)

Gift Messages:

If in the checkout your gift can't be saved due an error (for example sender name to long); the interface updates, the user thinks all is well, there are no error logs but the gift message is ignored.

Cause:
https://github.com/magento/magento2/blob/2.4.0/app/code/Magento/GiftMessage/Model/GiftMessageManager.php#L87

Why are the exceptions caught, in the example case the method calling the add function has a try-catch as well.

Proposed solution

1. Don't try catch \Exception if possible
2. When catching an \Exception at least log it
3. When catching an exception and creating a new one, add it to it's parents.

How hard is it to handle an exception properly It's not done properly in so many places.

We are miserable since magento2 came out; hating our profession. Can't we get these small things fixed?

---

Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.

• Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.

Additional info

One way to reproduce the issue is to add a throw new CouldNotSaveException(__("Show custom error.")); code to the /app/code/Magento/GiftMessage/Model/GiftMessageManager.php file

and try to imitate an exception

Preconditions

Magento 2.4-develop
Allow Gift Messages is enabled

Steps to reproduce

1. Add a throw new CouldNotSaveException(__("Show custom error.")); code to the /app/code/Magento/GiftMessage/Model/GiftMessageManager.php file
2. Add a Product to Cart
3. Type long name with different characters into From and To fields under Gift Options
4. Update

Expected Result

An exception message in logs

Actual Result

No exception message in logs

[m2-assistant]

Hi @oliverde8. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[ihor-sviziev]

Hi @oliverde8,
I do agree with you, it have to be at least logged somewhere.

This issue should be split to two things that basically quite related to each other:

1. Do not throw an \Exception

Unfortunately we still have such places in Magento2 code that need to be refactored, examples

magento2/app/code/Magento/UrlRewrite/Helper/UrlRewrite.php

Line 38 in 9544fb2

throw new \Exception(__('Anchor symbol (#) is not supported in request path.'), self::VERR_ANCHOR);

magento2/lib/internal/Magento/Framework/Search/Request/Mapper.php

Line 116 in 89d3cda

throw new \Exception('Query ' . $queryName . ' does not exist');

In order to not introduce new such places we have a static test for that https://github.com/magento/magento-coding-standard/blob/1b65bd6062c7f1da40c70b64aa3970acd4563087/Magento2/Sniffs/Exceptions/DirectThrowSniff.php

2. Empty catch statements

The same situation with empty catch statements - unfortunately we have them and should clean them up, examples

magento2/app/code/Magento/Integration/Model/OauthService.php

Lines 152 to 153 in 9544fb2

	} catch (\Exception $e) {
	}

magento2/dev/tests/integration/testsuite/Magento/Catalog/_files/product_system_attribute_rollback.php

Lines 32 to 34 in 9544fb2

	// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch
	} catch (\Exception $e) {
	}

We even have the static test that fails in case of empty catch section, example #25279 (comment)

---

Feel free to create PRs for fixing such places!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

[ihor-sviziev]

I think we should keep this issue open