-
Notifications
You must be signed in to change notification settings - Fork 22
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
Assorted error handling fixes #1641
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release #1641 +/- ##
=============================================
+ Coverage 17.74% 19.65% +1.90%
- Complexity 2316 2375 +59
=============================================
Files 43 45 +2
Lines 8758 8930 +172
=============================================
+ Hits 1554 1755 +201
+ Misses 7204 7175 -29 ☔ View full report in Codecov by Sentry. |
36de778
to
f3abbf1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
I also see these errors when running tests. Not introduced by this PR, just sharing because it seems related
wp_die() called
Message: Error retrieving Mailchimp campaign.
Title: WordPress › Error
Args:
response: 500
code: wp_die
exit: 1
back_link:
link_url:
link_text:
text_direction: ltr
charset: UTF-8
additional_errors: array (
)
newspack_newsletters_mailchimp_error:
@adekbadek @leogermani please look at #1642 to see if this fixes the timeouts we've been seeing on the |
Co-authored-by: leogermani <leogermani@automattic.com>
@leogermani – the tests issue should also be resolved by this PR. In the last few failed CI test-php jobs, the offending test was |
🎉 This PR is included in version 3.1.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [3.2.0-alpha.2](v3.2.0-alpha.1...v3.2.0-alpha.2) (2024-09-04) ### Bug Fixes * error handling, tests, CI builds ([#1641](#1641), [#1642](#1642)) ([791fdc3](791fdc3)) * handle missing Mailchimp API key ([83a0d6f](83a0d6f)) * **phpcs:** specify path in custom ruleset ref ([#1637](#1637)) ([28f5b50](28f5b50)) ### Reverts * "chore(deps-dev): bump @wordpress/browserslist-config from 6.5.0 to 6.6.0" ([f08f271](f08f271))
🎉 This PR is included in version 3.2.0-alpha.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Adds error handling in a few places to avoid triggering PHP Fatals, and updates the tests so they don't rely on network.
Newspack_Newsletters_Constant_Contact_SDK::get_contact
can returnfalse
orWP_Error
in case of failure, and theupsert_contact
method was not handling the latter case.test_render_embed_blocks
test not to rely on the networkSubmitting as a hotfix because the lack of error handling triggers PHP Fatals, which pollute our logs.
How to test the changes in this Pull Request:
Other information: