Error calling a view with non-numeric argument, e.g. "Content: ID"

Thank you for the patches. Bugs in Drupal Core require steps to reproduce. Please do not remove that tag until providing the steps to reproduce.

The MR/solution is trivial enough to not warrant credit.

We need steps to reproduce, so we may consider adding tests.

Aside: empty is typically too naive, we need to be using narrower type checking.

Marking postpone until we have some steps to reproduce this on a clean Drupal install, usually this is just hiding a problem upstream.

The question to answer is why this would ever be NULL, is it NULL in a valid scenario or due to another problem.

@shrikant.dhotre, thanks. Can you add more detail? Does the error happen when creating the view or when viewing the page. There is an example of the detail needed in Document steps to reproduce an issue.

I did try to test, creating a view with a integer numeric field with a name 'field_2'. I did not get an error when creating the page or viewing it.

I have also restored the standard template to make this issue easier to work on.

In my opinion, this question arose because of a change in this place:
public string $operator;

The value NULL for this variable is assigned here if the passed string does not match the regular expression

The simplest solution seems to be to allow the variable $operator to be NULL

I my case this error was occurred when in custom code are passing the substitution placeholder to the numeric filter.

      $selection_handler_options = [
        'view' => [
          'view_name' => 'view_name_id',
          'display_name' => 'display_name_id,
          'arguments' => ['substitution', $entity->id()],
        ],
      ];

I ran into this error after upgrading a Drupal 9 site to 10.

I agree with #16: originally this site started out as an early Drupal 8 site. Somehow, one of the views had its pager's offset configuration unset. Setting the offset value to '0' fixed the issue for me.

I would suggest just to allow NULL for \Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator

@@ -108,7 +108,7 @@ abstract class ArgumentPluginBase extends HandlerBase implements CacheableDepend
   /**
    * The operator used for the query: or|and.
    */
-  public string $operator;
+  public ?string $operator;

Agree with #26...

In my case, the error is thrown in a view that is embedded in a webform. The view arguments are passed by ajax and come from an element that is on the same page of the view. So in some conditions the view gets NULL being passed as an argument. In this case, somehow NULL gets to the operator. Implemented the suggestion suggested in #26 and it seems to work fine.

This is the patch based on #26 which works for my case.

Hello all,

Just to inform you, I've locally tested the patch from comment #28 and can verify that it is effective.

I encountered the following error:
TypeError: Cannot assign null to the property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\ArgumentPluginBase->unpackArgumentValue() (line 1310 of core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php).

The setup is as follows:
- Drupal version: 10.1.4
- PHP Version: 8.1.18

Thank you.

We are also facing this issue patch #29 and #28 solved it.

I know this needs steps to reproduce but this property can be null and needs to be treated like that. Either by this patch or by allowing it to be null.

Patch #28 also solved the issue on our site.

+1 on patch #28, fixes the problem.

Patch #28 and #29 both fix my issue. I'm not sure which is the best approach.

In my case, default contextual filters that happen to be null because the field was not required were causing this fatal error.

Patch #29 worked for me.

#29 worked for me. Thanx!

#29 fixed the issue for me too.

After a thorough review of the available information and attempts to understand this issue, I have concerns regarding the feasibility of reproducing the error, particularly since it seems to predominantly affect sites that have been upgraded from Drupal 8 or Drupal 9.

From what I've gathered, this error does not readily manifest in fresh installations of Drupal 10, which suggests that it might be tied to specific configurations or legacy data from older versions. This makes it challenging to replicate the conditions necessary to observe the error in a controlled environment.

Patch #29 does not add any new functionality, but merely checks whether $break->operator is set. Which appears to have successfully resolved the issue for many who have applied it, indicating its effectiveness in addressing this specific error.

Given these circumstances, I would like to propose a consideration for the integration of Patch #29 into core.

I have this error in a view which is using search_api.

TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\ArgumentPluginBase->unpackArgumentValue() (line 1310 of modules/views/src/Plugin/views/argument/ArgumentPluginBase.php).

Drupal\search_api\Plugin\views\argument\SearchApiStandard->fillValue() (Line: 160)
Drupal\search_api\Plugin\views\argument\SearchApiStandard->query() (Line: 1126)
Drupal\views\ViewExecutable->_buildArguments() (Line: 1282)
Drupal\views\ViewExecutable->build() (Line: 392)

It's a very simple view which just does a fulltext search and shows the search excerpt. It broke when upgrading to Drupal 10. Whilst patch #29 would fix it, I agree with #38 that the fix only treats the symptom but not the root cause. So I investigated some more and in my case I found it was due to a contextual filter which had a default value of a fixed value of an empty string, but which had a validator for "Content" that looked for the ID of a node and handles multiples with "One or more IDs separated by , or +".

So people should check the contextual filters and make sure an impossible situation hasn't been created where a blank value is validated. One could argue that the validator shouldn't cause an exception on the site, however Drupal had been instructed to do something that didn't make sense, and simply ignoring it would mean the mistake is never found.

I had the same error as @ovanes, #30.

TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\ArgumentPluginBase->unpackArgumentValue() (line 1303 of core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php).

This came from a view that takes a node id as a contextual argument, and it was being passed a random argument. When that argument included a non-numeric character, the error was triggered.

The cause of all this is the function breakString($str) in HandlerBase.php, which returns $operator as a string ('or' or 'and') when $str is alphanumeric and returns it as null when $str contains a non-alphanumeric character. I don't see the null return behavior documented anywhere; is it important to retain it? If not, the fix would be to change breakString() so it always returned a string for $operator - either 'and', which seems to be the default, or simply a null string, which would at least not cause TyeError issues.

I solved my problem by adding numeric validation to the contextual argument, but it seems like this should be cleaned up.

I confirm the description of #39, with a similar situation:

I have a site started with Drupal 8, upgraded at some point to 9 and recently to 10.

On that site, there are some views of nodes with a contextual filter "content:ID", whose validator is "content type". It accepts one or multiple ids.

Those views are sometimes rendered programmatically, with the following code:

    $view = [
      '#type' => 'view',
      '#name' => $view_id,
      '#display_id' => $view_display,
      '#arguments' => $nids,
    ];
    $render = $this->renderer->render($view);

Where $nids sometimes may be an empty string - if no value is provided.
Until Drupal 9 this worked, but since Drupal 10 I had to put a check in case $nids is an empty string, because that would cause the error.

I also agree with #39 considerations: it was poor custom code that made Drupal doing something that should not be possible.
However, hiding that error would also hide eventual code\configuration errors without having a way to realize it.

The patch here should be converted to an MR - and the last run on this issue had over a thousand test failures (probably a DrupalCI issue but we need a recent run on gitlab CI to commit it).

Additionally, this approach is just hiding the error without informing admins. I think we could log an error with the view name and display to tell the site admin something is wrong - maybe they can resave or rebuild the view or something.

I came up with this error today. In my project, we're calling views programmatically and some contextual filters started with a comma ",". In 9.5.x this was no problem, but we upgraded to 10.2 few days ago and got this. I first tried to re-save the view but didn't have any changes in the config.

I think we need to add a check that if is_null($break->operator), it means the filter values used to call the view are invalid and return that error to the user

Also, for the actual problem, if the property 'break_phrase' of the 'nid' argument is set to 'true', it will produce this error when using invalid arguments. The 'break_phrase' property in the UI is "Allow multiple values" in "More" when editing a contextual filter

I'm uploading a test patch that reproduces this error

I'm moving the work to https://git.drupalcode.org/project/drupal/-/merge_requests/6774. The other MRs can be closed

@pcambra did you maybe forget to push to the issue branch? Otherwise there's nothing to review

sorry, I was testing patch in #29 (which solves my issue) and changed this by mistake

OK. but this patch just hides the error, not solving the actual issue. The problem originates from the fact that an argument in your view is invalid

Updated title to be more descriptive.

Patch #28 alone works for me. Thanks.

In my case, i had a string value with the character "/" on it, so the regular expression was not picking it up and causing the operator to be NULL.

I think the main issue comes from HandlerBase::breakString, which should allways return an operator, even if none of the regular expressions apply. In that case, the value returns as an empty array, so i believe it should be safe to return any of the operators for that matter.

This issue does not only happen with numeric arguments, but also with string arguments that contain characters not present in the regular expression, so the fix should not focus on the numericArgument plugin.

Please review the new branch '3353786-breakstring-add-default-operator'. Setting the issue as needs review.

PD: Another different issue, which will require a different issue to address, is broadening the regular expression to allow for more special characters to be present, like the one that was causing the issue for me "/".

Issue summary does not match the current solution.

Quick glance though not sure that's the approach. The doc does not mention it could be null. Seems like if it becomes null that's the bug.

Yes exactly
It becomes null because the argument starts with an invalid character and since the "Use multiple values" option is enabled, it tries to filter it for "+"(OR) and ","(AND) characters, anything else is invalid.

I think we need to re-evaluate the scope of the issue, per #57 comment, to filter out invalid characters in arguments (as an option in UI maybe, to cover all use-cases), and keep this feature disabled, in order not to break anything during updates. If this option is disabled, we show an error to the user, as is the current proposed resolution, and if it's enabled, no harm done

I believe that the fix should be merged, so the breakString does not return a NULL operator, which causes a fatal php error, and then we can create a follow-up issue to discuss the acceptable characters in the provided values, as #59 suggests.

Here my 2 cents:
The operator class variable is part of ArgumentPluginBase - which as the name says is a base class.
As it behaves now, it is completely up to whoever inherits ArgumentPluginBase what to do if the operator is not set.
And I think this should be the case - introducing a default value would change the base to something opinionated which would be quite a change.
The same basic principle seems to apply to HandlerBase::breakString() - if it can't decide what to do based on the input given - it essentially declares that by returning NULL for the operator.
As such I'd advocate to allow NULL as value for the operator class variable.
However, I'd change the current MR code slightly to :

public ?string $operator = NULL;

Simply because this would fly with phps declare(strict_types=1); already and will never produce an exception like Typed property ArgumentPluginBase::$operator must not be accessed before initialization and NULL will still pass all potential isset() tests.

Now the mentioned issues with HandlerBase::breakString() not properly handling some strings is kind of unrelated to this issue - there are other issues related to this e.g. #672606

My use case is a combination of Search API & Views Reference where tokens are used to build views arguments. Which is not related to any broken D9 D10 upgrade path but simply how one can configure a views setup.

I also happened to face this issue. In my case, I was passing in some arguments to the view through code, but I got that error. After reading this thread, I started to make some tests. In my case, the value and operator were not being processed correctly, that meant the argument value I was passing into the view was not entering either if in HandlerBase::breakString. I was passing in 3500 IDs, which, as a concatenated string, became a 21.763-length string. If I cut back in the length, from 3500 ids to 3200, the string became around <20.000 length. For the first long string, there was no match, for the second shorter string, however, there was a match. So, everything pointed out to the length of the string. I updated the regex to be a bit more performant, and now my use case does work.