-
Notifications
You must be signed in to change notification settings - Fork 293
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
Uncaught exception during setup when missing site_id
or site_code
#5412
Comments
site_id
or site_code
@felixarntz we currently don't catch this anywhere it's used so it seems like it shouldn't throw, especially as there doesn't seem to be a clear action to take if it would. Thoughts? |
@aaemnnosttv We don't catch it anywhere because not passing any of the two arguments there would be an error on our part. The old In other words, this should still throw because it indicates a critical incorrect usage within the codebase itself. If this gets thrown without another plugin being involved, it's something critical we have to investigate and fix. Basically, you could consider this similar to a PHP error where we are not passing a required method parameter. |
@felixarntz that makes sense. It looks like the only way this error would be possible is here by passing an empty site-kit-wp/includes/Core/Authentication/Setup.php Lines 236 to 238 in a16315e
I can't see how this could happen though as it seems like it would require the site code exchange to fail somehow without throwing. Essentially not having credentials by the end of the After playing with this a bit, I found it is possible to force this using the filter: add_filter( 'googlesitekit_oauth_secret', function () {
$web = [
'client_id' => '',
'client_secret' => '',
];
return compact( 'web' );
} ); I suppose someone could be using it improperly – can you see any other way this could happen? |
I don't think that can happen from the API response. So I would think some code is doing something wrong there indeed. I think we should investigate this further, but I don't think it is a critical bug, I assume there is some third-party code involved in this. 🤔 |
Hi @aaemnnosttv if you are not actively working on this, could you un-assign yourself so someone else can pick up? Thanks! |
@jamesozzie have there been any recent reports of this? It sounds like something we'd definitely want to fix but one that isn't common or easy to reproduce under reasonable circumstances. |
@aaemnnosttv We've encountered no recent reports of this. We only came across a single time in total. |
Happening for me too! |
Hi @alex-maereanu. We'd be happy to look into your case if you'd like to open a support topic over on the plugins WordPress support forums. Please include all relevant details in order for us to assist. This may include sharing your Site Health information, so we can check your site and your current AdSense status. If we can find any useful additional insights we can also add them to this GitHub issue, which may be useful given it's likely others are encountering the same. I'll keep an eye out for your topic. Thank you. |
Encountered this issue today with a new site. |
@strarsis As directly above, we'd be happy to assist you with this if you'd like to open a support topic over on the plugins WordPress support forums. If the experience you're facing is the same as described in the issue we may need to make some changes within the plugin, hopefully however, we can determine more when working with you from a support topic. |
@jamesozzie: I created a new support topic in the WordPress support forum for the plugin: |
Today I've experienced the same issue for a website we are building. We see the same errors provided in the WordPress support ticket above. For reference, here is the one we've:
Because of curiosity I attempted to debug it a little bit with the codebase alongside. What I've discovered is that we got stuck here: https://github.com/google/site-kit-wp/blob/develop/includes/Core/Authentication/Google_Proxy.php#L138. I also can confirm we have a |
@andizer Really appreciate you sharing your insights into this. If you'd like to open a support topic we'd be happy to work with you on this. After opening a support topic you may wish to share any invalid response, maybe via the Log HTTP Requests plugin or XHR. If sharing this, please share via this form only (referencing your own support topic). The request for sitekit.withgoogle.com/o/oauth2/site/](http://sitekit.withgoogle.com/o/oauth2/site/ in particular may be of use. In relation to the If we can determine additional insights based on your setup, with the details you provide, we can revisit this GitHub issue and add notes based on your experience. |
@andizer @jamesozzie To clarify, only one of I'm unsure what leads to the exception being thrown as the logic itself looks right. The exception is only thrown if neither of the parameters is present. |
The error still persists on the affected site. This is the URL that causes the exception during the Google Site Kit set-up: |
@strarsis As the error persists for you, I've reopened your support topic. Based on what you shared above, it looks like you have your WordPress files in a wp folder. If you had Site Kit setup previously without this wp directory, and then moved your files without resetting the plugin, it's possible to encounter errors. I'll leave a note in your support topic and we can communicate from there. |
Good point. Though the WordPress file always had been in that folder. Other sites using the same layout work correctly. |
Bug Description
One user in the support forums is encounter problems when trying to set up the plugin. They were at one stage encountering a set up loop, I suspect redirecting after signing in with their Google account.
At present their set up experience is described as following:
The user has tried the same with Site Kit as the only plugin active (via the Health Check & Troubleshooting plugin)
We did have a similar error that was reported as a bug previously, since addressed (#3830), with compile errors now including SK information (#3896).
Screenshots
The full video of this experience can be found within the impacted users Site Health information report.
Additional Context
Insights & Troubleshooting Checks performed
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Google_Proxy::setup_url
should no longer throw exceptionsImplementation Brief
Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: