Add option to log deprecation errors to prepare for Drupal 11

Created on 7 January 2019, almost 6 years ago
Updated 2 August 2024, 5 months ago

Problem/Motivation

Dries had this IMHO great idea in https://twitter.com/Dries/status/1081582626560008192 I'd also love to see a "Log deprecated code" setting in /admin so you can also identify deprecated code without having to run tests. which could be a good way to turn on and let it run for a while to audit for problems. Proper tools to vet the results would still be important (dependencies will throw these errors, some errors should not be fixed too soon without core updates, etc), but we need some way to collect the data first.

User stories from #20:

  1. As a site owner, I want to see if my site can be upgraded to Drupal 9 (e.g. there should be no deprecations).
  2. As a developer, I want to see what deprecations I have to remove. Because I have to manage custom code, I'd like to know the module, file name and line number of the deprecated code.

Proposed resolution

Add logic to _drupal_error_handler_real to used the configuration values to determine if a message should be logged.
The configuration allows one to

  • Enable/disable the logging of deprecation messages
  • Filter messages bases on file path
  • Filter messages by the message text

Remaining tasks

Get testing to work and likely add more test cases.

  • Discuss log retention around these messages, these may generate a lot of logs fast, but at the same time aggregate results out of it would be *really* useful to prioritise work on a site.
  • In separate issue: discuss ways to improve making sense of this data, currently messages logged this way don't really stand out in logs and hard to filter for (they show up alongside other 'php' sourced errors and no further filter to filter to deprecated things or to aggregate them and see volumes):

User interface changes

after

API changes

None.

Data model changes

None.

Release notes snippet

TBD.

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Baseย  โ†’

Last updated 1 day ago

Created by

๐Ÿ‡ญ๐Ÿ‡บHungary Gรกbor Hojtsy Hungary

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡ญ๐Ÿ‡บHungary mxr576 Hungary
  • heddn Nicaragua

    Seems like a new title is in order.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly plach Venezia

    What about using PHP settings to make the behavior configurable? We could make this opt-in by assuming a FALSE default and punt aggregation/filtering to a follow-up issue :)

  • ๐Ÿ‡จ๐Ÿ‡ญSwitzerland berdir Switzerland

    I closed ๐Ÿ“Œ Allow to display/log E_USER_DEPRECATED messages Closed: duplicate as a duplicate. Note that the other issue also tried to handle skipped deprecations, which is IMHO crucial for this, as some of them have been logged a _lot_ historically and would completely spam that log, making it rather useless.

  • ๐Ÿ‡ญ๐Ÿ‡บHungary mxr576 Hungary

    Removed redundant tag

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly plach Venezia

    @Berdir

    Note that the other issue also tried to handle skipped deprecations, which is IMHO crucial for this, as some of them have been logged a _lot_ historically and would completely spam that log, making it rather useless.

    Fair, what about making those configurable via a setting as well? We could hardcode a default if the setting is NULL.

  • First commit to issue fork.
  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber

    Here's some initial work on this: https://git.drupalcode.org/issue/drupal-3024296/-/compare/11.x...3024296...
    Not sure if filename - based deprecation level (all, contrib + custom, custom only) filtering is reliable enough, namespace is not for sure as we cannot distinguish contrib from custom at all there. Let's get this moving in any case :)

  • Merge request !6699#3024296: Enable deprecation logging. โ†’ (Open) created by Graber
  • Pipeline finished with Failed
    10 months ago
    Total: 169s
    #99335
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber

    Created a MR against 11.x, no tests yet as it'll be nice to receive feedback first and some recommendations what'd be the best way to test (Unit?).
    Marking for initial review.
    Adding a patch to lock at this state as well to avoid composer issues if the MR gets updated as well.

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    MR appears to have some failures.

  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber

    @smutsgrave, yes, I'm aware of that - typo in a comment. Nevertheless, I asked for a small review at this stage to ensure the direction is correct before more hours are spent on this. Can we have it?

  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Go for it. Haven't checked the follow up tag need either.

  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber

    Thanks, received some internal feedback on this and thought about it a bit in the meantime.
    TODO:
    - fix typo obviously
    - remove deprecation level in favor or a boolean switch and ignored file path patterns (*/core/* and */vendor/* by default)
    - we should probably only log deprecations instead of displaying them on pages.

  • Pipeline finished with Failed
    10 months ago
    Total: 484s
    #100412
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber

    Right, now work has to be done to update existing tests and write new ones. Moving this to review again so we can see if this has chances of merging at this state assuming tests are handled.

  • ๐Ÿ‡ต๐Ÿ‡ฑPoland Graber
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave
  • Status changed to Needs work 9 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Seems @alexpott left some feedback on the MR.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    @alexpott and I have looked at this, removing the tag

  • First commit to issue fork.
  • Pipeline finished with Failed
    6 months ago
    Total: 674s
    #204229
  • Pipeline finished with Failed
    6 months ago
    Total: 500s
    #210805
  • Pipeline finished with Failed
    6 months ago
    Total: 600s
    #210839
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone

    Started a test but in the test there is no logging of the deprecation message. I don't see why that is. Anyone?

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    it may help to catch deprecations from upcoming PHP release ๐ŸŒฑ [META] Make Drupal 10.3/11 compatible with PHP 8.4 Active

Production build 0.71.5 2024