Not sure if this was officially released but in any case I added a link
Thanks for reporting!
Already resolved by humans
It looks like the sessions table was removed from system module in Drupal 10.3.0, according to https://www.drupal.org/node/3431286 → - it's now created on demand by the session handler.
So yes, we now need to check if the table exists. This also means that the code that gets a description of the sessions table from system schema (for use on the config form) only works on older versions of Drupal.
What session handler do you have in use? Ideally, we could provide ip_anon support for removing IP addresses from alternative session storage
Opened 🐛 Actually add service alias for Symfony\Component\EventDispatcher\EventDispatcherInterface Active to add the alias correctly
mfb → created an issue.
Removing the ability for loggers to depend on config would be a big trade-off for the contrib loggers that I maintain - there would no longer be a nice, friendly UI for admins to tweak logging without having to tinker with the settings file or yaml files. It certainly would help resolve circular dependency hell, though...
@farse on the current 7.x branch I am still seeing a ServiceCircularReferenceException if I install symfony_mailer module and core workspaces_ui module along with any logger that uses config, such as core syslog module or Raven module.
Given that the issue also affects core syslog module, I would say the issue should ideally be resolved in either symfony_mailer or core?
There might be some workaround we could do in Raven module, but I'm not sure off hand what that would be, aside from no longer using config..
For future reference, this ideally would've been filed as a new issue, as it was a new bug in the 7.x branch, but in any case thanks for reporting
Please test the MR - I believe this should resolve the issue by refactoring config to "properly" override (rather than having a special snowflake config service).
For anyone seeing a circular reference, please provide steps to reproduce from a fresh install of Drupal core. This would include which contrib module(s) and how they need to be configured to reproduce the bug. (If a custom module is required to reproduce the bug, please upload or link to some example reproduce code.)
For extra bonus points you could open a merge request with a failing test :)
@dfisher I still don't know how to do this. I may have missed something obvious as I don't use this module per se, was just trying to make my contrib module compatible with it.
Move feature request to current dev branch.
I believe you would now need to call something like Sentry.addIntegration(integration: Sentry.replayIntegration());, as we now use a callback function for drupalSettings.raven.options.integrations rather than a simple array.
Move feature request to current dev branch
Move feature request to current dev branch
Move feature request to current dev branch
In a followup issue we may need to add a log filtering feature? Although, developers can do their own filtering via Sentry callbacks.
For example, maybe by default, logs could honor the same "ignored channels" and "ignored messages" that errors do, with a checkbox to capture all logs, including ignored channels and ignored messages? There might even be a use case for logs having its own lists of ignored channels and ignored messages.
Actually fixed on the 7.x branch
Actually fixed on the 7.x branch
Actually fixed on the 7.x branch
mfb → changed the visibility of the branch 3541436-backport-3 to active.
mfb → changed the visibility of the branch 3541436-backport-3 to hidden.
This likely needs a backport
I discovered an API method we can use to convert the description markup to plain text: PlainTextOutput::renderFromHtml(). And for some reason, the default title is empty string, but the default description is null. So, we need to handle that potential null by only using the description if it's present.
Opened up a MR for review. Since title is apparently used in de-duplicating items, i thought it might be a good idea to save the truncated description instead of an empty title? But also, since a NULL or empty title would be valid (if saved programmatically, for example), I also patched the label() to use the truncated description, if necessary. Since the description can be HTML-encoded markup, we have to strip tags and HTML-decode the description before using it as the title, otherwise the markup would be displayed to the end user.
Setting to needs work because this isn't really fixed by the patch, yet :)
I updated the issue summary with a pretty-important remaining task: Figure out exactly how to handle RSS items that are missing a title or have an empty title, due to the item title being used in the UI, e.g. when listing items, and by the label() method; all this stuff still needs to work for items without a title!
Far as I can tell you're good here, the dependency is on ^2.17, which allows any later 2.* version to be installed, including the current version, which is 2.24.0
Tests are failing, but I don't think it's related to this MR, as tests are also failing on the 2.x-dev branch
So I think this could be RTBC?
Added a commit to resolve a deprecation notice I've been hitting on a production site:
Deprecated function: Drupal\aggregator\ZfExtensionManagerSfContainer::setContainer(): Implicitly marking parameter $container as nullable is deprecated, the explicit nullable type must be used instead
As far as sustainability a few things come to mind:
- A file hash submodule could report duplicate files and even allow them to be merged.
- A "sustainability" contrib module could instruct site admins to install helpful modules and configure them more sustainably.
I don't intend to enable the dedupe validator by default. It is a complex feature that could be enabled site-wide, on a per-field basis, or programmatically; there are also the choices of "strict" dedupe and "original" dedupe. And I do consider it a "proof of concept" - sites may want to improve the dedupe UX.
@liam morland did you see my comments in #30? I feel like it would be helpful if this issue could decide to either trigger a deprecation if null is returned in cases where it is not documented to be possible, or change the documentation to allow null, despite the data model not allowing null.
In other words, my @todo was something to figure out in this issue, not a future issue.
We didn't make another issue AFAIK. The current state of the patch is "for review" to figure out next steps.
I could continue dealing with any security issues on the drupal 7 branch, but that's all I've had time for since taking on co-maintainer role, not other bugs or feature requests, or other branches.
Postponing on ✨ Provide the ability to instrument the queue system Active
I can't tell from your description exactly what happened. Maybe there was an error that lead to circular reference, and once you fixed it the circular reference disappeared? But it sounds like maybe there was no actual error, you were trying to do something totally valid which lead to the circular reference?
Either situation is not good...
Because of how the raven logger (unfortunately) has service dependencies, it's difficult to completely avoid the potential for circular references. But when they are found we do try to find a way to eliminate them.
It would be great to resolve this issue, because Bluesky RSS feeds do not have a title.
I'm reclassifying this as a bug, because it seems like a good idea to support Bluesky's RSS feeds.
Please provide steps to reproduce from a clean install of Drupal so we can look into this. For example, install module X, enable Y on the settings page, add Z to the settings.php file, etc.
Ok, glad that works for you.
I do think there is some potential for improvement here, like checking $context['@message'] for matching ignored message, if it exists, to make it easier for non-developers to ignore messages..
That said this module is really targeted at developers who can do a lot of customization in code...
Sorry for the miscommunication: Subscribing to the event and setting the before_send callback is something you should do in your custom code to provide the functionality you're looking for.
It should not be done in this module.
Thinking thru some pros and cons of various solutions for this:
If we check both the unformatted and formatted message in ignored_messages, then it would be easy to ignore specific errors. However, whenever the line number or absolute path of the file changes, the error would no longer be ignored. Therefore, this would be a pretty brittle solution - it doesn't seem particularly great to me
Another idea would be to check both the unformatted message and, if it exists, $context['@message']. This would be a more general rule, ignoring new cases of the error that appear, regardless of where it appears. So, it seems more useful as a filter, although certainly could cause people to unintentionally miss new errors.
We intentionally match the generic unformatted/uninterpolated message because that is often necessary. For example, we don't want to have to ignore a message for each user logging in, but rather for "[user] logged in". So, we can't make this change.
To filter events the way you want you'll need to write a little code to set the before_send callback option, which allows you to provide arbitrary custom logic for filtering events; the ignore_exceptions option is also available.
It looks like Drush has its own queue runner code, so would need to duplicate the event dispatching code?
These would be my proposed events to enable instrumenting:
When an item is added to a queue: ItemQueuing, ItemQueued
When a queue item is being processed: ItemProcessing, ItemExceptionOccurred, ItemProcessed
Ready for review - this removes some potential edge case behavior changes where null would have been cast to 0.
But, the methods are not documented to return null. So, I think either a deprecation should be triggered if null is returned, or the documentation could be changed to allow null, despite the data model not allowing null.
Updated the change record
As for the instrumentation, this shouldn't have any influence on application code. I have been evaluating contrib-auto-pdo and this is recording spans for database queries / statements from the very beginning of a request / response cycle if configured properly. No need for events or other application level code in that case.
Very nice! I might play around with this myself. Note the only reason this works is because of the opentelemetry PHP extension.
I'd like to see it built in to core - presumably by dispatching events - so no overriding would be necessary. So next step could be to file a core feature request
Did you have a concept for how to instrument the queue API? I didn't see anything obvious last I looked, unlike Laravel which fires events as jobs are processed. Might be necessary to alter/override the queue service and/or work on adding instrumentability to core queue?
This module already includes HTTP request instrumentation. Are you finding that it's not working?
Drupal 11 is already supported
Altering JS configuration is already documented to some extent in the README file, but feel free to make improvements
Then sounds like this can be closed? Feel free to re-open if you have more info
@joseph.olstad I wasn't able to reproduce this issue; please write a failing test or update the steps to reproduce
Drupal 11 compatible release is already available
Yeah only bad code interpreting the feed could be a danger here, because untrusted third party strings need to be just that, untrusted, and run thru something like HTML Purifier or the filter built into drupal core; i.e. security issues shouldn't be possible regardless of what's in a third party string.
Closing this as no one has worked on it in a while. Feel free to re-open if someone wants to contribute a merge request.
Closing this since no one worked on it. Happy to review merge requests but this didn't have one. Feel free to re-open if someone wants to contribute :)
Closing this because that project was discontinued
Already released for Drupal 11
This module already works on Drupal 11
Ok great, I'll roll a new release soon.
It would also be nice to fix the underlying bug in BigPipe module, but I haven't figured that out yet. At the very least I should be able to write a failing test..
@joel_guesclin Good news, I think I was able to work around the bug in core - please try the merge request version of piwik_noscript, or apply the patch.
I did some refactoring to use automatically-created placeholders, rather than manually-created placeholders - this bypasses the bug in BigPipe module. The automatically-created placeholder is a bit silly, because it means we are executing javascript to place a noscript element, but this code path allows BigPipe to apply drupalSettings correctly.
@catch I did some more debugging on this and determined that this bug only affects manually created, non-markup placeholders, which are essentially skipped - although still broken - by BigPipe.
Many placeholders will not be affected by this bug because they use automatically-created placeholders ('#create_placeholder' = TRUE)
Ok, so we don't yet know why your drupalSettings is getting loaded in the <head> as opposed to in the <body> - it wouldn't be the "solo" theme since that doesn't have any libraries that specify both header: true and dependencies: core/drupalSettings
You would probably need to give me all your installed modules (contrib and custom) so that I could try to reproduce it - or figure out on your own why drupalSettings is getting loaded in <head>
@joel_guesclin if you're able to track down a contrib module you're using with a library that specifies header: true and depends on core/drupalSettings, that would seem to be what's triggering this issue - at least in my testing.
AFAICT, the AJAX system seems to be able to merge new settings from an AJAX response into the global settings just fine, it's just not happening when BigPipe processes placeholders.