search

google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.23k stars 286 forks source link

Tag Manager error during new container setup: Error: Returned an error response for your request #1817

Closed jamesozzie closed 3 years ago

jamesozzie commented 4 years ago

Bug Description

When using the plugin to setup a new Tag Manager container the below error appears when clicking on "Confirm & Continue" Error: Returned an error response for your request. This seems to occur only when trying to create a new container for the second time (after disconnecting the Tag Manager module)

image

Steps to reproduce

  1. Start the Tag Manager connection process
  2. Select the option to "Setup a new container", either web or AMP
  3. Click on "Confirm & Continue"
  4. Error appears

Additional Context

Site Health info ``` ### wp-core ### version: 5.4.2 site_language: en_US user_language: en_US permalink: /%postname%/ https_status: true user_registration: 0 default_comment_status: open multisite: false user_count: 5 dotorg_communication: true ### wp-paths-sizes ### wordpress_path: /var/www/vhosts/beijing.ie/httpdocs wordpress_size: 96.22 MB (100894666 bytes) uploads_path: /var/www/vhosts/beijing.ie/httpdocs/wp-content/uploads uploads_size: 37.40 MB (39214146 bytes) themes_path: /var/www/vhosts/beijing.ie/httpdocs/wp-content/themes themes_size: 19.17 MB (20096230 bytes) plugins_path: /var/www/vhosts/beijing.ie/httpdocs/wp-content/plugins plugins_size: 231.01 MB (242230756 bytes) database_size: 84.15 MB (88235503 bytes) total_size: 467.94 MB (490671301 bytes) ### wp-active-theme ### name: Twenty Seventeen (twentyseventeen) version: 2.2 (latest version: 2.3) author: the WordPress team author_website: https://wordpress.org/ parent_theme: none theme_features: automatic-feed-links, title-tag, post-thumbnails, menus, html5, post-formats, custom-logo, customize-selective-refresh-widgets, editor-style, editor-styles, wp-block-styles, responsive-embeds, starter-content, custom-header, widgets theme_path: /var/www/vhosts/beijing.ie/httpdocs/wp-content/themes/twentyseventeen ### wp-themes-inactive (9) ### ampface: version: 1.0.0, author: James Osborne AMPFace1.6: version: 1.6.0, author: James Ozzie Osborne Astra: version: 2.3.2, author: Brainstorm Force (latest version: 2.4.5) Bezel: version: 0.1.6, author: SimpleFreeThemes Blocksy: version: 1.7.10, author: CreativeThemes (latest version: 1.7.22) ampface - blogger child theme: version: 1.0, author: admin GeneratePress: version: 2.4.2, author: Tom Usborne Twenty Nineteen: version: 1.4, author: the WordPress team (latest version: 1.6) Twenty Twenty: version: 1.2, author: the WordPress team (latest version: 1.4) ### wp-mu-plugins (1) ### Health Check Troubleshooting Mode: author: (undefined), version: 1.7.1 ### wp-plugins-active (6) ### AMP Debug Validation Request: version: 0.2, author: Weston Ruter, Google Classic Editor: version: 1.5, author: WordPress Contributors Debug Bar: version: 1.0.1, author: wordpressdotorg Health Check & Troubleshooting: version: 1.4.4, author: The WordPress.org community Site Kit by Google: version: 1.12.0, author: Google WpSimpleTools Log Viewer: version: 1.0.2, author: WpSimpleTools ### wp-plugins-inactive (57) ### Advanced Custom Fields: version: 5.8.12, author: Elliot Condon AMP: version: 1.5.5, author: AMP Project Contributors AMP - sidebar - jamesozz.ie: version: 1.0, author: James Osborne AMP Blocks: version: 1.6.3, author: Magazine3 AMP Sidebar Hamburger Menu: version: 1.2.0, author: James Osborne Atomic Blocks - Gutenberg Blocks Collection: version: 2.8.5, author: atomicblocks Autoptimize: version: 2.7.5, author: Frank Goossens (futtta) Clear OpCache Simple: version: 0.1.0, author: Felix Arntz, Google Cloudflare: version: 3.5.1, author: Cloudflare, Inc. Cloudflare Flexible SSL: version: 1.3.0, author: One Dollar Plugin CoBlocks: version: 2.0.3, author: GoDaddy CSS Only Back to Top Button: version: 1.0.0, author: James Osborne Download Monitor: version: 4.4.3, author: Never5 Duplicator: version: 1.3.36, author: Snap Creek Easy Digital Downloads: version: 2.9.23, author: Easy Digital Downloads Error Log Monitor: version: 1.6.10, author: Janis Elsts EWWW Image Optimizer: version: 5.5.0, author: Exactly WWW (latest version: 5.6.0) Flexible Map: version: 1.17.1, author: WebAware Glue for Yoast SEO & AMP: version: 0.7, author: Joost de Valk Google Language Translator: version: 6.0.6, author: Translate AI Multilingual Solutions Gutenberg: version: 8.5.1, author: Gutenberg Team iThemes Security: version: 7.7.1, author: iThemes Jetpack by WordPress.com: version: 8.6.1, author: Automattic (latest version: 8.7.1) LiteSpeed Cache: version: 3.2.4, author: LiteSpeed Technologies Log HTTP Requests: version: 1.2, author: FacetWP, LLC Login With Ajax: version: 3.1.10, author: Marcus Sykes OneSignal Push Notifications: version: 2.1.2, author: OneSignal Query Monitor: version: 3.6.0, author: John Blackbourn Really Simple SSL: version: 3.3.4, author: Really Simple Plugins Relevanssi: version: 4.7.2.1, author: Mikko Saari Sidebar Manager: version: 1.1.3, author: Brainstorm Force Site Kit by Google Staging Proxy: version: 1.0.0, author: Google Site Kit by Google Tester: version: 1.0.0, author: Google Site Kit Test: version: 1.0.0, author: rafaucau Statify: version: 1.7.2, author: pluginkollektiv Sucuri Security - Auditing, Malware Scanner and Hardening: version: 1.8.24, author: Sucuri Inc. Temporary Login Without Password: version: 1.6.3, author: StoreApps W3 Total Cache: version: 0.14.2, author: BoldGrid (latest version: 0.14.3) Webcraftic Clearfy – WordPress optimization plugin: version: 1.6.8, author: Webcraftic WooCommerce Admin: version: 1.3.1, author: WooCommerce WooCommerce Stripe Gateway: version: 4.5.0, author: WooCommerce WooSidebars: version: 1.4.5, author: WooCommerce Wordfence Login Security: version: 1.0.5, author: Wordfence Wordfence Security: version: 7.4.9, author: Wordfence WordPress RSS Feed Retriever: version: 1.6.2, author: Theme Mason WP-Optimize - Clean, Compress, Cache: version: 3.1.2, author: David Anderson, Ruhani Rabin, Team Updraft WP Async JavaScript: version: 2.20.03.01, author: Frank Goossens (futtta) wpCentral: version: 1.5.3, author: Softaculous Ltd. WP Cerber Security, Antispam & Malware Scan: version: 8.6.5, author: Cerber Tech Inc. WP Chat App: version: 2.0, author: NinjaTeam WP Fastest Cache: version: 0.9.0.8, author: Emre Vona WPForms Lite: version: 1.6.1, author: WPForms WP User Frontend: version: 3.3.1, author: weDevs WP YouTube Lyte: version: 1.7.13, author: Frank Goossens (futtta) Yet Another Related Posts Plugin (YARPP): version: 5.2.1, author: YARPP Yoast Duplicate Post: version: 3.2.5, author: Enrico Battocchi & Team Yoast Yoast SEO: version: 14.6, author: Team Yoast (latest version: 14.6.1) ### wp-media ### image_editor: WP_Image_Editor_Imagick imagick_module_version: 1656 imagemagick_version: ImageMagick 6.7.8-9 2019-02-01 Q16 http://www.imagemagick.org imagick_limits: imagick::RESOURCETYPE_AREA: 7 GB imagick::RESOURCETYPE_DISK: -1 imagick::RESOURCETYPE_FILE: 768 imagick::RESOURCETYPE_MAP: 7 GB imagick::RESOURCETYPE_MEMORY: 4 GB imagick::RESOURCETYPE_THREAD: not available gd_version: bundled (2.1.0 compatible) ghostscript_version: 9.07 ### wp-server ### server_architecture: Linux 3.10.0-957.5.1.el7.x86_64 x86_64 httpd_software: Apache php_version: 7.2.15 64bit php_sapi: cgi-fcgi max_input_variables: 1000 time_limit: 30 memory_limit: 256M max_input_time: 60 upload_max_size: 126M php_post_max_size: 22M curl_version: 7.29.0 NSS/3.44 suhosin: false imagick_availability: true server-headers: server: nginx date: Tue, 21 Jul 2020 17:55:20 GMT content-type: text/html; charset=UTF-8 x-powered-by: PHP/7.2.15 expires: Wed, 11 Jan 1984 05:00:00 GMT cache-control: no-cache, must-revalidate, max-age=0 htaccess_extra_rules: true ### wp-database ### extension: mysqli server_version: 5.5.60-MariaDB client_version: mysqlnd 5.0.12-dev - 20150407 - $Id: 3591daad22de08524295e1bd073aceeff11e6579 $ ### wp-constants ### WP_HOME: undefined WP_SITEURL: undefined WP_CONTENT_DIR: /var/www/vhosts/beijing.ie/httpdocs/wp-content WP_PLUGIN_DIR: /var/www/vhosts/beijing.ie/httpdocs/wp-content/plugins WP_MAX_MEMORY_LIMIT: 256M WP_DEBUG: false WP_DEBUG_DISPLAY: true WP_DEBUG_LOG: false SCRIPT_DEBUG: false WP_CACHE: false CONCATENATE_SCRIPTS: undefined COMPRESS_SCRIPTS: undefined COMPRESS_CSS: undefined WP_LOCAL_DEV: undefined ### wp-filesystem ### wordpress: writable wp-content: writable uploads: writable plugins: writable themes: writable mu-plugins: writable ### google-site-kit ### version: 1.12.0 php_version: 7.2.15 wp_version: 5.4.2 reference_url: https://beijing.ie amp_mode: no site_status: connected-site user_status: authenticated active_modules: site-verification, search-console, adsense, analytics, pagespeed-insights, tagmanager required_scopes: openid: ✅ https://www.googleapis.com/auth/userinfo.profile: ✅ https://www.googleapis.com/auth/userinfo.email: ✅ https://www.googleapis.com/auth/siteverification: ✅ https://www.googleapis.com/auth/webmasters: ✅ https://www.googleapis.com/auth/adsense.readonly: ✅ https://www.googleapis.com/auth/analytics.readonly: ✅ https://www.googleapis.com/auth/tagmanager.readonly: ✅ search_console_property: https://beijing.ie/ adsense_account_id: pub-346••••••••••••• adsense_client_id: ca-pub-346••••••••••••• adsense_account_status: approved adsense_use_snippet: yes analytics_account_id: 1721••••• analytics_property_id: UA-1721••••••• analytics_profile_id: 2234••••• analytics_use_snippet: yes tagmanager_account_id: none tagmanager_container_id: none tagmanager_amp_container_id: none tagmanager_use_snippet: yes ```

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

QA Brief

  1. Start the Tag Manager connection process
  2. Select the option to "Setup a new container", either web or AMP
  3. Click on "Confirm & Continue"
  4. There should be a text field that is pre-populated with the container name.

You should be able to submit the form as-is, as we should add numbers to the container name if one already matches the generated name, so you shouldn't need to modify the container name to submit it.

Also test the same workflow (steps 2-4 above) but when editing an existing, already setup Tag Manager account/module in Site Kit. The container name field should appear during setup and editing.

Changelog entry

jamesozzie commented 4 years ago

I've been able to reproduce this on another site following the steps below:

  1. Create new GTM account
  2. Connect the Tag Module module, when connecting select the "Set up a new container"
  3. Setup works fine (with the new containers appearing when checking GTM directly)
  4. Disconnect the Tag Manager Module
  5. Attempt setup once more
  6. Select the already created account and then choose to create a new container (web or AMP, same results)
  7. Error appears

Video / Example

cole10up commented 4 years ago

Worked with @jamesozzie on this using http://www.orient.one/

Was added to the tag manager account, and ran through a test trying to setup a new container. Wasn't able to setup a new container and experienced a looping loading bar spamming 400 errors in the console.

image

Added @aaemnnosttv to the site and tag manager.

aaemnnosttv commented 4 years ago

@cole10up @jamesozzie - the problem is that Tag Manager does not allow 2 different containers to have the same name. Currently the container names are generated from the site name with no ability to change it (other than to change your site name first; at least temporarily).

@felixarntz @marrrmarrr - I think we need to offer the ability for the user to customize the container name. Similar to what we did for Analytics view creation, only it's more important in this case because Analytics allows multiple views with the same name, but GTM does not allow multiple containers with the same name. It's not a great experience for the user either as this is a predictable error from the client side. I'll add some ACs here as a first draft and assign to you for refinement.

felixarntz commented 4 years ago

@tofumatt One thing that's somewhat missing from the IB is considering AMP. Implementing the ContainerNameTextField is different from e.g. #716 here since in case of AMP another setting will be used (and for secondary AMP, if both containers are set to create a new one, we'd need to show two different name fields).

I suggest we implement both a WebContainerNameTextField and AMPContainerNameTextField components. Internally they could use a similar ContainerNameTextField component that receives the few differing pieces via props. This is also how e.g. the container dropdowns are implemented for Tag Manager.

The other thing that's missing (maybe that's obvious, but probably should still be mentioned) is that these components should be integrated not only into SettingsForm, but also into SetupForm. (Also the QA brief should probably state to test this in both module setup and module settings.)

It would be a nice UX if, when selecting "set up new container" and there is already a container with the same name, we add a number or something to the container name so the user isn't immediately presented with a name they need to change.

Let's not implement this here, I think we should be more explicit and go with the same approach we're doing for Analytics, except here we'd make it more of an error message than just a warning, since here it would actually block the user from proceeding.

felixarntz commented 4 years ago

IB ✅

aaemnnosttv commented 3 years ago

@felixarntz @eugene-manuilov it seems the API has changed recently to no longer return an error for a request to create a container with a duplicate name. I tested this today for the release and found that I was able to successfully create a new container multiple times without changing the site name (as was necessary before).

image

I still think this issue is relevant, but we should redefine it to be similar to the behavior of Analytics View creation, which does not block the user for a duplicate view name; only warns. Currently the issue here is defined in a way which blocks the user from creating a container with a duplicate name (which made sense before) but that part seems to no longer be relevant.

aaemnnosttv commented 3 years ago

@felixarntz @eugene-manuilov taking another look at this, I found the above behavior only happens when creating a web container. Creating an AMP container raises the same error that we saw before. That makes me think that this may be a bug with the API, so we should probably stick to the original definition here and require a new name to always be unique and otherwise block the user from proceeding.

felixarntz commented 3 years ago

@aaemnnosttv @eugene-manuilov I just noticed there seems to be a problem with the new behavior here, at least in Storybook: The button here is disabled although everything seems to be fine. Screenshot 2020-10-29 at 13 40 01

I'm not sure this is only a result of an incorrect setup in Storybook for this story or whether something is actually broken in production code, but I think this should be addressed.

eugene-manuilov commented 3 years ago

Good catch, @felixarntz. Yes, it's a bug. I have created a new PR that fixes it: https://github.com/google/site-kit-wp/pull/2294

wpdarren commented 3 years ago

QA Update: In process

  1. Started the Tag Manager connection process https://d.pr/i/OVQ8dn
  2. Setup a new container with Web
  3. Confirmed the Connection and Container https://d.pr/i/usgWek
  4. The container name and account appears within SK https://d.pr/i/ZbTKeW
  5. Disconnected Tag Manager from SK https://d.pr/i/Ptd7o5
  6. Started the Tag Manager connection process for the same account
  7. Set up a new container with Web and used the same container name as before https://d.pr/i/RnBXte
  8. Tag Manager set up with new container https://d.pr/i/6mQgeR. No errors appearing in the console.
  9. Confirmed that user can only use a container name once https://d.pr/i/sX6bpf
  10. Related to the issue reported by Felix. I was able to set up a new container on a current account. The confirm and continue button is not inactive https://d.pr/i/rqAxKe

@cole10up an observation: when I edit the tag manager in settings I can see a JS error message in the console. https://d.pr/i/jS3Aux I am not overly familiar with tag manager, so wasn't sure. Any thoughts? Update: this JS error is expected, so not an issue to report. Confirmed with Evan.

Environment:

SiteKit Version | 1.19.0 (in developer branch) PHP Version | 7.3.21 WordPress Version | 5.5.2

cole10up commented 3 years ago

@wpdarren Looks good and confirmed the error you're seeing is expected. Thank you!