Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle missing metadata #73

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Handle missing metadata #73

merged 1 commit into from
Nov 7, 2022

Conversation

kadamwhite
Copy link
Contributor

@kadamwhite kadamwhite commented Nov 4, 2022

In certain situations, especially with migrated legacy content, an image URL may be sent to tachyon for an image which doesn't exist on disk and may not have any stored metadata. In this case, because we never check that the meta array is well formed and didn't return as "false", several log warnings will be emitted:

Trying to access array offset on value of type bool | …achyon-plugin/inc/class-tachyon.php#631
Trying to access array offset on value of type bool | …achyon-plugin/inc/class-tachyon.php#632

(Observed in Altis 12 on PHP 8—it's very noisy.)

Almost all of our calculations in this if block assume that meta exists and ws returned as an array. If that's not true, we should move on to compute entirely based on the provided arguments.

Relates to #69 (and probably #60)

In certain situations, especially with migrated legacy content, an image
URL may be sent to tachyon for an image which doesn't exist on disk and
may not have any stored metadata. In this case, because we never check
that the meta array is well formed and didn't return as "false", several
log warnings will be emitted:

```
Trying to access array offset on value of type bool | …achyon-plugin/inc/class-tachyon.php#631
Trying to access array offset on value of type bool | …achyon-plugin/inc/class-tachyon.php#632
```

Almost all of our calculations in this `if` block assume that meta
exists and ws returned as an array. If that's not true, we should move
on to compute entirely based on the provided arguments.
Copy link
Contributor

@mikelittle mikelittle left a comment

Choose a reason for hiding this comment

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

Looks good to me

@kadamwhite kadamwhite merged commit 8202841 into master Nov 7, 2022
@kadamwhite kadamwhite deleted the handle-missing-meta branch November 7, 2022 18:19
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.

3 participants