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

Event observers fire from disabled modules #9948

Open
scottsb opened this issue Jun 14, 2017 · 15 comments
Open

Event observers fire from disabled modules #9948

scottsb opened this issue Jun 14, 2017 · 15 comments
Labels
Area: Upgrades - Upgrade Compatibility Tool bug report Component: DB Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@scottsb
Copy link
Member

scottsb commented Jun 14, 2017

Magento will still fire observers for disabled modules.

Besides simply triggering unwanted functionality, this can also potentially cause fatal errors if the module was disabled before the install scripts ever ran, as the observer will expect the module to be installed (and any schema/data changes to be applied), which it won't be in this case.

Preconditions

Magento 2.1.3

Steps to reproduce

  1. Add the code for a module that has a setup script with schema changes used by an observer (e.g., Magento_AdminNotification), but do not yet run setup:upgrade.
  2. Disable the module.
  3. Run setup:upgrade. Because the module is disabled, schema changes will not be made (correctly so).
  4. Perform action on site triggering observer that assumes database schema (e.g., log into admin when there is a pending notification).

Expected result

Observer is not fired, since module is disabled.

Actual result

Observer is fired, causing a potentially fatal error if it assumes schema changes have occurred.

@Stas94
Copy link

Stas94 commented Jul 3, 2017

@scottsb Please add more details to your description of the steps you followed when identifying this issue. Screenshots or logs would be helpful, too.

@scottsb
Copy link
Member Author

scottsb commented Jul 5, 2017

Is there a particular area you want clarification on? I already provided the steps to reproduce. (For what it's worth, the example I cite of Magento_AdminNotification isn't just hypothetical; it's the actual module we had disabled that caused us to run into this, so you could do the same.)

@ghost
Copy link

ghost commented Jul 5, 2017 via email

@scottsb
Copy link
Member Author

scottsb commented Jul 5, 2017

I did just clarify the problem statement in the issue. The fundamental bug is simply that event observers in disabled modules still run. My steps to reproduce illustrate how this can lead to a fatal error, though obviously there are many ways this could fail since code is running that's supposed to be disabled.

@veloraven veloraven added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed Progress: needs update labels Jul 25, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Oct 13, 2017
@magento-engcom-team
Copy link
Contributor

@scottsb, thank you for your report.
We've created internal ticket(s) MAGETWO-71009 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 13, 2017
@thiagolima-bm
Copy link
Member

I am working on it #SQUASHTOBERFEST

@ghost ghost moved this from Ready for Dev to Dev in Progress in Community Backlog Apr 20, 2020
@shikhamis11
Copy link
Member

I also tried to reproduce this issue on magento 2.3.4 instance but it is not reproducible now
@scottsb can you confirm the same ?

@sidolov sidolov added this to Ready for Grooming in Low Priority Backlog Sep 3, 2020
@m2-community-project m2-community-project bot moved this from Ready for Grooming to Dev In Progress in Low Priority Backlog Sep 3, 2020
@sidolov sidolov added this to Ready for Development in Low Priority Backlog Sep 24, 2020
@m2-community-project m2-community-project bot moved this from Ready for Development to Dev In Progress in Low Priority Backlog Sep 24, 2020
@ghost ghost unassigned shikhamis11 Oct 8, 2020
@m2-community-project m2-community-project bot moved this from Dev In Progress to Ready for Development in Low Priority Backlog Oct 8, 2020
@sidolov sidolov moved this from On Hold to Confirmed in Issue Confirmation and Triage Board Oct 20, 2020
@ghost ghost removed this from Dev in Progress in Community Backlog Oct 20, 2020
@ghost ghost removed this from Ready for Development in Low Priority Backlog Oct 20, 2020
@ghost ghost added Issue: On Hold Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: On Hold labels Oct 20, 2020
@sdzhepa sdzhepa added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Nov 5, 2020
@magento-engcom-team magento-engcom-team added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Nov 30, 2020
@m2-community-project m2-community-project bot added this to Ready for Development in High Priority Backlog Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Upgrades - Upgrade Compatibility Tool bug report Component: DB Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
High Priority Backlog
  
Ready for Development
Development

No branches or pull requests