Error response for merchant center link/create operation. by ankitrox · Pull Request #3157 · woocommerce/google-listings-and-ads

ankitrox · 2025-11-26T03:25:47Z

Changes proposed in this Pull Request:

Closes: https://linear.app/a8c/issue/GOOWOO-339/merchant-center-account-creationlinking-fails-insert-link

Replace this with a good description of your changes & reasoning.

Screenshots:

Detailed test instructions

You may need to checkout to test/GOOWOO-214-integration branch to test these changes on FE. This feature branch is merged in the integration branch.

Make sure test proxy is running.
Go to step 1 of the onboarding, make sure Jetpack and Google accounts are connected and Ads and MC cards are visible, but are not connected.
Try connecting Ads and MC accounts one by one, the error would be displayed in corresponding cards as shown in screenshot.

Note: We also have corresponding E2E tests for these scenarios as well as for accounts auto-creation scenrio in integration branch given above.

Additional details:

Changelog entry

codecov · 2025-11-26T03:29:54Z

Codecov Report

❌ Patch coverage is 41.26984% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.6%. Comparing base (c906f68) to head (110e124).
⚠️ Report is 71 commits behind head on feature/GOOWOO-214-better-user-guidance.

Files with missing lines	Patch %	Lines
src/MerchantCenter/AccountService.php	12.5%	14 Missing ⚠️
src/API/Google/Middleware.php	47.8%	12 Missing ⚠️
src/Ads/AccountService.php	8.3%	11 Missing ⚠️

Additional details and impacted files

@@                             Coverage Diff                              @@
##             feature/GOOWOO-214-better-user-guidance   #3157      +/-   ##
============================================================================
- Coverage                                       67.7%   66.6%    -1.1%     
- Complexity                                         0    5116    +5116     
============================================================================
  Files                                            359     869     +510     
  Lines                                           5835   26206   +20371     
  Branches                                        1446    1454       +8     
============================================================================
+ Hits                                            3950   17455   +13505     
- Misses                                          1711    8573    +6862     
- Partials                                         174     178       +4

Flag	Coverage Δ
js-unit-tests	`67.5% <ø> (-0.2%)`	⬇️
php-unit-tests	`66.4% <41.3%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/API/Site/Controllers/Ads/AccountController.php	`97.9% <ø> (ø)`
...e/Controllers/MerchantCenter/AccountController.php	`100.0% <100.0%> (ø)`
src/Ads/AccountService.php	`91.8% <8.3%> (ø)`
src/API/Google/Middleware.php	`89.8% <47.8%> (ø)`
src/MerchantCenter/AccountService.php	`88.8% <12.5%> (ø)`

... and 503 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mukeshpanchal27

Thanks for the PR. Could you please add testing instructions to the PR?

It would also be great if you include the Proxy testing: https://github.com/woocommerce/google-listings-and-ads/tree/develop/tests/proxy so we can test it easily.

mukeshpanchal27 · 2025-12-08T15:00:47Z

src/API/Google/Middleware.php

+			if ( $e instanceof BadResponseException ) {
+				$raw = json_decode( $e->getResponse()->getBody()->getContents(), true );
+				if ( is_array( $raw ) && ! empty( $raw['errors'] ) && is_array( $raw['errors'] ) ) {
+					foreach ( $raw['errors'] as $err ) {
+						if ( isset( $err['code'], $err['message'] ) ) {
+							$errors[ (string) $err['code'] ] = (string) $err['message'];
+						}
+					}
+				}
+			}


This code is duplicated across the files. Please factor it out into a shared utility function.

mukeshpanchal27 · 2025-12-08T15:01:08Z

src/MerchantCenter/AccountService.php

+			if ( $e->getPrevious() instanceof BadResponseException ) {
+				/** @var BadResponseException $prev */
+				$prev    = $e->getPrevious();
+				$body    = method_exists( $prev, 'getResponse' ) && $prev->getResponse() ? (string) $prev->getResponse()->getBody() : '';
+				$decoded = json_decode( $body, true );
+				$error   = is_array( $decoded ) ? ( $decoded['error'] ?? [] ) : [];
+				$message = is_array( $error ) && isset( $error['message'] ) ? (string) $error['message'] : $e->getMessage();
+				throw $this->prepare_exception(
+					$message,
+					[
+						'code'  => 'API_ERROR',
+						'error' => $error,
+					],
+					$e->getCode() ?: 400
+				);
+			}


Same as previous message

…O-339-merchant-linking-error-response

ankitrox · 2025-12-12T17:14:16Z

Thank you @mukeshpanchal27 for reviewing the PR and adding the feedback.

I have updated the proxy to accommodate the changes and also updated the test instructions to test the PR changes.

Over to you for another review.

mukeshpanchal27 · 2025-12-18T04:20:36Z

When i test this with proxy i get below errors:

ankitrox · 2025-12-18T06:58:03Z

@mukeshpanchal27 Thanks for looking into it.
Can you please confirm if you are testing on the test/GOOWOO-214-integration branch?

mukeshpanchal27

Thanks for the update. It looks solid overall. I’ve left a few questions. Let me know if you need any clarification.

mukeshpanchal27 · 2025-12-22T04:09:01Z

src/API/Google/Middleware.php

+					}
+				}
+			}
+			throw new Exception( $message, $status, $e );


Why do we reference $e instead of $errors, considering $errors is derived from $e?

We can skip adding the error object completely. $errors can't be added as third parameter needs to be of type Throwable and $errors is array.

mukeshpanchal27 · 2025-12-22T04:17:35Z

src/API/Site/Controllers/MerchantCenter/AccountController.php

+					$status  = $e->getCode() ?: 400;
+					$error   = is_array( $data ) ? ( $data['error'] ?? [] ) : [];
+					$message = is_array( $error ) && isset( $error['message'] ) ? (string) $error['message'] : $e->getMessage();


This should be moved inside the if condition, since this variable only needs to be set when the error code is not API_ERROR.

Thanks, nice observation. I updated it.

mukeshpanchal27

Thanks @ankitrox for the updates. The changes look good in my testing.

Make sure to merge develop branch so unit tests get passed.

…O-339-merchant-linking-error-response

Error reesponse for merchant center link/create operation.

9516d2f

github-actions bot added type: enhancement The issue is a request for an enhancement. changelog: add A new feature, function, or functionality was added. labels Nov 26, 2025

ankitrox added 5 commits November 26, 2025 12:10

Fix: phpcs issues.

85cfdee

Fix: phpcs issue.

5c92a25

Update the approach to send REST error.

9c858b7

Remove unnecessary files.

e5f957e

Remove redundant comments.

7c168d7

asvinb changed the title ~~Error reesponse for merchant center link/create operation.~~ Error response for merchant center link/create operation. Dec 5, 2025

mukeshpanchal27 reviewed Dec 8, 2025

View reviewed changes

ankitrox added 4 commits December 11, 2025 12:26

Merge branch 'feature/GOOWOO-214-better-user-guidance' into add/GOOWO…

abc05f2

…O-339-merchant-linking-error-response

Add proxy handlers for connection requests.

a22c6d2

Update response structure.

9c7eff8

Extract common utility function for error handling.

2e531ae

ankitrox force-pushed the add/GOOWOO-339-merchant-linking-error-response branch from 992f35c to 2e531ae Compare December 12, 2025 17:46

vikrampm1 requested a review from mukeshpanchal27 December 17, 2025 16:14

ankitrox added 2 commits December 18, 2025 21:45

Fix: php error.

23a55e4

Update ads link/create functionality.

d7edcb7

mukeshpanchal27 reviewed Dec 22, 2025

View reviewed changes

ankitrox added 2 commits January 5, 2026 13:07

CR feedback.

bdd5e7a

Fix: phpcs issue.

7ff6e95

mukeshpanchal27 approved these changes Jan 7, 2026

View reviewed changes

Merge branch 'feature/GOOWOO-214-better-user-guidance' into add/GOOWO…

110e124

…O-339-merchant-linking-error-response

Conversation

ankitrox commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions

Additional details:

Changelog entry

Uh oh!

codecov bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

ankitrox commented Dec 12, 2025

Uh oh!

mukeshpanchal27 commented Dec 18, 2025

Uh oh!

ankitrox commented Dec 18, 2025

Uh oh!

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

ankitrox Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

ankitrox Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ankitrox commented Nov 26, 2025 •

edited

Loading

codecov bot commented Nov 26, 2025 •

edited

Loading