Skip to content

Add guard to anonymous function#423

Merged
afragen merged 1 commit intofairpm:release_1.3.0from
afragen:fix-412
Jan 13, 2026
Merged

Add guard to anonymous function#423
afragen merged 1 commit intofairpm:release_1.3.0from
afragen:fix-412

Conversation

@afragen
Copy link
Contributor

@afragen afragen commented Jan 13, 2026

Fixes #412

Somehow release->artifacts->package is null and sending to pick_artifact_by_lang() returns a type error. Adding a guard to the anonymous function should mitigate this issue.

Fixes fairpm#412
Signed-off-by: Andy Fragen <andy@thefragens.com>
@github-actions
Copy link
Contributor

@afragen afragen added this to the 1.3 milestone Jan 13, 2026
Copy link
Member

@costdev costdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine as-is, though I'd also be curious whether the value being stored in the transient should be checked. If, for example, a release document should always have artifacts->package, if that's missing from a release document, we may consider skipping it, or possibly erroring, if needed.

@afragen
Copy link
Contributor Author

afragen commented Jan 13, 2026

If the transient didn't return an array wouldn't the error be different?

@afragen afragen merged commit f47e82c into fairpm:release_1.3.0 Jan 13, 2026
3 checks passed
@costdev
Copy link
Member

costdev commented Jan 13, 2026

If the transient didn't return an array wouldn't the error be different?

It would, though I wasn't referring to the CACHE_RELEASE_PACKAGES array itself, I was referring to each release document inside it. If a release document doesn't have ->artifacts->package, should it even be stored in the cache array?

The cache is set here:

set_site_transient( CACHE_RELEASE_PACKAGES, $releases );

So if get_latest_release_from_did() returns a release document without artifacts->package, is there any reason to add this invalid WP "package" to the cache?

Moreover:

  1. I don't quite understand how this situation occurred. How was a DID detected, but the package didn't have any package artifacts?
  2. In cases such as malformed metadata, I don't think the code should be getting this far. I think it may make sense to return a WP_Error object from get_latest_release_from_did() if a release doesn't have a package artifact, then add is_wp_error() checks in all the appropriate places. Testing required of course.

@afragen
Copy link
Contributor Author

afragen commented Jan 13, 2026

Is this a product of the transient being an empty array?

@costdev
Copy link
Member

costdev commented Jan 13, 2026

Nope, look at line 726 in the file touched by this PR - so the transient isn't an empty array. It's due to hitting a missing package artifact when looking at each release document inside that array.

@afragen afragen deleted the fix-412 branch January 14, 2026 00:34
@afragen
Copy link
Contributor Author

afragen commented Jan 14, 2026

That's essentially what the guard protects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐞 Bug Report: WP locks up when trying to update a theme (from wp.org)

2 participants