Unexpected short‑circuit return values from add_metadata() and update_metadata()

[marian1]

Documented Behavior

WordPress documentation states that:

• add_metadata() returns int|false — the new meta ID or false on failure.
• update_metadata() returns int|bool — a new meta ID, true on update, or false on failure/no change.

Actual Behavior

However, both functions can be short‑circuited via their respective filter hooks (add_{$meta_type}_metadata, update_{$meta_type}_metadata) by returning any non‑null value. That value is cast to a boolean value before returning it by update_metadata() and returned directly by add_metadata(), possibly violating the documented return type and causing unexpected behaviors.

Specifically:

• If a filter hooked to add_{$meta_type}_metadata returns a non‑null value (also non‑boolean types including positive integers suggesting success), add_metadata() returns that value—even if the metadata was never added.

• If a filter hooked to update_{$meta_type}_metadata returns a non‑null value, update_metadata() casts it to bool and returns it—potentially returning true when no update occurred.

These behaviors do not align with the documented behaviors and introduce ambiguous and misleading return values.

Proposed Fix

In add_metadata(), change:

<?php
if ( null !== $check ) {
    return $check;
}

to:

<?php
if ( null !== $check ) {
    return false;
}

This ensures short‑circuiting always indicates failure and matches the documented return type.

In update_metadata(), similarly replace:

<?php
if ( null !== $check ) {
    return (bool) $check;
}

with:

<?php
if ( null !== $check ) {
    return false;
}

This avoids returning true in scenarios where metadata wasn't updated.

These changes ensure reliable, semantically clear return behavior that aligns with documentation and expected usage.

[abcd95]

Thanks, @marian1, for the ticket.

I was able to reproduce the issue and can confirm that this behavior occurs. I agree that this type of behaviour is definitely misleading in the sense that return values can be unexpected.

The thing with the suggested approach to always return false is - Always returning false on short circit assumes all filters intend to block the operation, which may not be accurate. Existing plugins might rely on the current behavior where filters can perform metadata operations themselves and indicate success.

I'd like to propose an alternative approach that maintains backward compatibility while fixing the core problem -

For add_metadata() - 

if ( null !== $check ) {
    if ( is_int( $check ) || false === $check ) {
        return $check;
    }

    _doing_it_wrong(
        __FUNCTION__,
        'Filter hooks for add_metadata() must return an integer (the new meta_id) or false.',
        '6.8.something'
    );

    return false;
}

A similar thing can also be done for the update_metadata() function as well. Let me know if this works well for you.

[peterwilsoncc]

The filters system in WordPress is really powerful, this can be both an advantage and a disadvantage as developers can make mistakes.

It's not possible to change the return value in each of these cases to false as it assumes developers are using the pre-flight filters exclusively to prevent the insertion of meta data. In some cases, plugins use the filter to move the meta data to custom meta tables and do need to return the ID of the inserted row, see ​EDD's back-compat code for an example.

@abcd95 I'm hesitant to throw a _doing_it_wrong as in that example for a couple of reasons:

• it suggests the error is with the WP function rather than a developers filter, the error message would need to reference the function returning the incorrect value and if multiple filters are added to the hook, it's not possible to know which is causing the error
• it also adds logs to the end-user's site when they may not be able to take action to recover from the error

That said, I'd like to think about it further and consider ways to implement it so developers who do make an error are informed. I'll seek some input from others.

[marian1]

I agree, filters are powerful. Perhaps this issue can be addressed in the next major release. Third-party code should not be permitted to alter the return type of core functions, and ideally should not be permitted to change the semantic meaning of returned values.

In the meantime, it might be worthwhile to reconsider the documentation. At present, the function descriptions give the impression that the returned values always have clear semantic meaning. However, if the function is short-circuited, this is not necessarily the case.

It is also worth noting that the filter documentation for add_{$meta_type}_metadata states, "Returning a non-null value will effectively short-circuit the function." This could encourage the use of non-boolean return values. Additionally, the documented @param null|bool $check contradicts the intended return values of add_metadata() by allowing true. And, the phrase "Whether to allow adding metadata for the given type." is misleading because true is then most likely read as "Yes, allow adding metadata for the given type" when true effectively short-circuits the function.

The additional information in the documentation of the ​update_{$meta_type}_metadata hook states "The filter must return a null value (the value of $check) if the data is be saved to the database. If it returns anything else, the update_metadata function (and therefore the update_{$meta_type}_metadata filter) will return what the filter callbacks return." which is incorrect, update_metadata returns bool for non null values. However, it is an accurate description of the add_{$meta_type}_metadata hook that lacks such additional information.

All of this, applies to the meta type specific wrapper functions of add_metadata() and update_metadata() as well.