Skip to content
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

Problem reinstalling, but no error logs #283

Open
bilogic opened this issue Mar 13, 2024 · 43 comments
Open

Problem reinstalling, but no error logs #283

bilogic opened this issue Mar 13, 2024 · 43 comments
Labels
bug Something isn't working

Comments

@bilogic
Copy link
Contributor

bilogic commented Mar 13, 2024

For bug reporting only! If you're posting a feature request or discussion, please ignore.

Expected Behavior

The (re)installation should proceed smoothly

Current Behavior

App was installed on this shop before. It was uninstalled and now trying to re-install again.
But the reinstalling keeps failing and returning to the install screen.

Failure Information

There are no error logs in laravel.log

Steps to Reproduce

Unknown, it has happened more than once and can be quite hard to reproduce

If users.deleted_at remains as null for whatever reasons when the app is uninstalled, this problem will surface
deleted_at only gets a value when Shopify's uninstall webhook succeeds, but webhooks can fail.

  1. Disable all webhooks in the shopify app (to simulate that Shopify has stopped retrying and all webhooks have failed)
  2. Install the app
  3. Uninstall the app (with webhooks disabled, users.deleted_at remains as null)
  4. Reinstall the app - unable to, leads to 404 on shopify backend

Hope this can be addressed.

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Package Version: v19.0.0
  • Laravel Version: v8.83.27
  • PHP Version: v8.1.27
  • Template Engine: Blade
  • Using a toolset (Docker, Laradock, Vagrant, etc.): n/a

Investigation to find a solution

Triggered javascript breakpoints

  1. /authenticate <= auth/full.blade.php

  2. click INSTALL

  3. /token <= layouts/default.blade.php, before createApp

  4. /token?embedded=1 <= layouts/default.blade.php, before createApp

  5. /token?embedded=1 <= token_handler.blade.php

  6. /billing <= billing/full.blade.php

  7. click APPROVE

  8. /token <= layouts/default.blade.php, before createApp

  9. /token?embedded=1 <= layouts/default.blade.php, before createApp

  10. /token?embedded=1 <= token_handler.blade.php.php

  11. / <= layouts/default.blade.php, before createApp

  12. click UNINSTALL

  13. click INSTALL

  14. /token <= layouts/default.blade.php, before createApp

  15. 404

@bilogic bilogic added bug Something isn't working unconfirmed Bug has not been reproduced yet labels Mar 13, 2024
@bilogic
Copy link
Contributor Author

bilogic commented Mar 13, 2024

I have to delete the user in order to break the install cycle, this package should have a way to detect and automatically break the cycle.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 14, 2024

In the VerifyShopify middleware

if ($tokenSource === null) {
    // Check if there is a store record in the database
    if ($this->checkPreviousInstallation($request)) {
        // Shop exists, token not available, we need to get one
        $a = $this->handleMissingToken($request); // this line causes the install loop
    } else {
        // Shop does not exist
        $a = $this->handleInvalidShop($request);
    }

    return $a;
}

@bilogic
Copy link
Contributor Author

bilogic commented Mar 15, 2024

It seems that we can also check for 401/403 failures under 2. Failing API requests https://www.littlestreamsoftware.com/labs/what-to-do-when-a-shopify-app-is-uninstalled/

I suppose this check should be part of checkPreviousInstallation so that handleInvalidShop gets called if we have been uninstalled but not properly informed.

@Kyon147
Copy link
Owner

Kyon147 commented Mar 19, 2024

@bilogic do you have the uninstall job set up

@Kyon147 Kyon147 closed this as completed Mar 19, 2024
@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

I do have an uninstall job set up, but I don't think that is the issue.

Webhooks can and do fail. How are we handling that today?

@Kyon147
Copy link
Owner

Kyon147 commented Mar 20, 2024

@bilogic the package does not handle failures automatically, you'd have to build this into the jobs you want to use from each webhook.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

@Kyon147

It is not about what the webhook does.

What if the network/server goes down for extended periods and the webhook does not reach our server?

@Kyon147
Copy link
Owner

Kyon147 commented Mar 20, 2024

If your server goes down then Shopify we try and resend the webhooks a number of times before it stops. It's all in their docs somewhere how often and how many tries they do.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

Yes. And when it stops and all attempts failed, it causes the infinite install loop, effectively permanently preventing this store from ever installing my app again.

I'm not sure why you see this is a non issue or why this package should not be responsible for this problem.

@Kyon147 Kyon147 reopened this Mar 20, 2024
@Kyon147
Copy link
Owner

Kyon147 commented Mar 20, 2024

Hi @bilogic

If the webhooks fail and we don't set the deleted at event and this is causing an error for the re-install then we need to refactor that re-install logic and use other fields to determine it potentially.

I'd be happy for you to open a PR if you wanted to submit something that could solve this edge case.

If not, I can take a look when I have some time.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

Ok, thanks, now we are on the same page. I will look into it and attempt a PR.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

Btw, I'm not that familiar with the Shopify API, which was why I relied on this package in the first place.

So offhand, do you have any opinions on how this can be addressed? e.g. which Shopify API should I attempt to try to determine if my app has been uninstalled etc?

@Kyon147
Copy link
Owner

Kyon147 commented Mar 20, 2024

If the store uninstalled the app, then you would not be able to use any API anyway.

We'd need to solve this using data we have in the users table at the time of install, that is the only data points we have when we don't have an api key to use.

Plus, we don't want to slow down the install by making requests externally really.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

Ok, noted on not making any external requests.

I did use an interactive debugger to try and understand the install lifecycle of this package, but I wasn't able to make much sense of it. Do you know of any references that is clear in explaining the installation, preferrable in a sequence diagram?

Or if you have an idea on how we can figure things out, I would be more than happy to just write and test the implementation.

@Kyon147
Copy link
Owner

Kyon147 commented Mar 20, 2024

There's no documented flow outside of what is already in the wiki which is a high level overview.

For the install, it's mainly in InstallShop https://github.com/Kyon147/laravel-shopify/blob/master/src/Actions/InstallShop.php

There's likely an error in the restore method on that file which is breaking if deleted_at is null but this is a educated guess as I'd need to go through the flow myself and debug it to be sure.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 20, 2024

Alright, let me check out InstallShop and update if I find anything interesting. Thanks!

@bilogic
Copy link
Contributor Author

bilogic commented Mar 21, 2024

Installation flow

Auth

  1. Managed (recommended)
  2. Auth code grant

It would appear that this package is performing utilizing 2

Token acquisition

  1. Token exchange (recommended)
  2. Auth code grant

Unable to tell which one this package is using yet

@bilogic
Copy link
Contributor Author

bilogic commented Mar 21, 2024

Updated the OP with a bunch of triggered javascript debugger breakpoints

@Kyon147 Kyon147 removed the unconfirmed Bug has not been reproduced yet label Mar 25, 2024
@Kyon147
Copy link
Owner

Kyon147 commented Mar 27, 2024

Just to update, I'm looking into this, this week.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 27, 2024

Wonderful, thank you so much. Please ask if you have trouble reproducing the problem.

@bilogic
Copy link
Contributor Author

bilogic commented Apr 3, 2024

Curious to know if there is progress on this. Thank you.

@Kyon147
Copy link
Owner

Kyon147 commented Apr 3, 2024

@bilogic not got to it just yet as I worked on testing the laravel 11 PR - will try and get to it this week

@thesunilyadav
Copy link

Hello @Kyon147 @bilogic

I'm encountering a similar issue and seeking an update. Currently, I'm facing two specific problems:

  1. AppUninstalledJob isn't being triggered: Despite expectations, AppUninstalledJob fails to execute as intended.
  2. After_authenticate_job issue: There's a similar hiccup with after_authenticate_job. After users uninstall and then reinstall the app, the job doesn't initiate. This issue has been experienced by approximately 10-15 users in the LIVE application recently.

=> To give you a clearer picture, here's how I've set up AfterAuthenticateJob:

'after_authenticate_job' => [
          [
            'job' => \App\Jobs\AfterAuthenticateJob::class,
            'inline' => true
        ],
    ],
 

Additional Information:

Package Version: 19.0.0
Laravel Version: 10.10.0
PHP Version: 8.1.0
Template Engine: Blade

I would greatly appreciate any assistance you can provide on this matter. Thank you in advance for your help! 🙂

@bilogic
Copy link
Contributor Author

bilogic commented Apr 14, 2024

@thesunilyadav

I faced your same issue too, but that issue isn't really a problem until it causes this problem that we are now trying to address.

I would suggest opening another issue in order for us to stay focused on this: how does this package self rectify an AppUninstalledJob after all retries have exhausted and still failed.

@Kyon147
Copy link
Owner

Kyon147 commented Apr 15, 2024

As @bilogic suggest can you open a separate issue, I've never seen the after auth fail for my apps personally but it is possible if the uninstall did not happen cleanly.

Hoping I can get the PR sorted asap.

@thesunilyadav
Copy link

Hello @Kyon147 @bilogic, I've created a new issue. Please feel free to take a look when you have a moment #Issue_298

@bilogic
Copy link
Contributor Author

bilogic commented Apr 22, 2024

@Kyon147 just curious if you managed to make any progress? Thank you!

@Kyon147
Copy link
Owner

Kyon147 commented Apr 22, 2024

Hey @bilogic

I think I have a working version on my local machine, apologies for the radio silence...partner is about to give birth so juggling things at the mo!

@bilogic
Copy link
Contributor Author

bilogic commented Apr 22, 2024

ah congrats! if you could, perhaps push a branch let me have a peek? ;) but don't let me inconvenience you thanks!

@bilogic
Copy link
Contributor Author

bilogic commented Apr 28, 2024

@Kyon147 possible to let me have a look? Thank you!

@bilogic bilogic closed this as completed May 2, 2024
@bilogic bilogic reopened this May 2, 2024
@bilogic
Copy link
Contributor Author

bilogic commented May 7, 2024

@Kyon147 hope everything is fine, I would like to help by having a test/look at your solution. Thank you!

@bilogic
Copy link
Contributor Author

bilogic commented May 13, 2024

@Kyon147 would love to help if you can share whatever you have at the moment. Thanks!

@Kyon147
Copy link
Owner

Kyon147 commented May 17, 2024

Hey @bilogic not forgotten about you or this ticket, just trying to decide on some bits for this before I submit a PR.

As the issue is based on the webhook, we need an "offline" solve for when a store is trashed but the package is not aware the store has been.

@bilogic
Copy link
Contributor Author

bilogic commented May 17, 2024

As the issue is based on the webhook, we need an "offline" solve for when a store is trashed but the package is not aware the store has been.

@Kyon147 thanks, well this set of criteria has gotten me stump.

Perhaps there is something in shopify's redirect that we could pick up on? Otherwise it is almost like being able to read shopify's database while being offline.

I did try earlier this year, but found nothing. What do you have already? Perhaps put it on a branch? Maybe we can build on each other's ideas.

@Kyon147
Copy link
Owner

Kyon147 commented May 18, 2024

The main issue is that once the app is uninstalled, if we miss the webhook, then that's it.

So my test at the moment is to trying and consistently, re-auth if for some reason we don't have a working api key, but a deleted_at column is still null and assume that we have a "dirty" store install and should treat it as a new install.

@bilogic
Copy link
Contributor Author

bilogic commented May 20, 2024

@Kyon147 sounds quite similar to what I had in mind previously.

But I wasn't able to determine if the API key I have is still working. Is this re-auth performed on every interaction with our UI? Or only during the installation request?

@bilogic
Copy link
Contributor Author

bilogic commented May 26, 2024

@Kyon147 would be great if I can see and build on what you already have thanks

@bilogic
Copy link
Contributor Author

bilogic commented Jun 3, 2024

@Kyon147 just checking in with you

@Kyon147
Copy link
Owner

Kyon147 commented Jun 15, 2024

Hey @bilogic

Apologies, new dad life, work and many other projects just sucking up all my time.

I'll try and get a PR out this weekend for you 👍

Thanks for being patient with me

@gofortiss
Copy link

Any news ?? Thanks !

@Kyon147
Copy link
Owner

Kyon147 commented Sep 24, 2024

Hey @bilogic

I think I have a solve for this, going to try and wrap it up by next week as I want to release it with some of the other PRs that are in master now.

Thanks for your paitence on this, just been trying to find the time to tackle it with enough time to focus on it

@bilogic
Copy link
Contributor Author

bilogic commented Sep 25, 2024

@Kyon147 do you have the specific commits that solves this problem? Thank you.

@VivekShingala
Copy link

Hi @Kyon147 did you already fix it?

Screenshot 2024-11-15 at 11 00 23 AM

Getting this page forever for the Shopify store whose uninstall webhook failed for any reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants