Skip to content

Commit

Permalink
[bugfix] Fix dereferencing ancestors on new status create (superserio…
Browse files Browse the repository at this point in the history
…usbusiness#2652)

* [bugfix] Pass `latest` to dereferenceThread instead of barebones status

* only mark status orphaned if visibility suggests parent is really deleted

* tone down "not deref'd" warnings, since they represent a legit visibility situation

* remove FAQ entry for "status not deref'd yet"
  • Loading branch information
tsmethurst authored Feb 18, 2024
1 parent 83a4adb commit 40f9eef
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 10 deletions.
4 changes: 0 additions & 4 deletions docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ Take a look at the [list of open bugs](https://github.com/superseriousbusiness/g

These warnings are caused by Mastodon forgetting to take query parameters into account when signing HTTP requests. Fixes on both Mastodon and GoToSocial sides are in the works, and other instance software is expected to follow.

## Warnings “status not yet deref'd” in the logs

The warning is mostly to say that the derived visibility of the given status may not be entirely accurate due to a parent not being dereferenced yet, so do not worry about it.

## Will you support tables in Markdown?

Not at the moment, as most clients handle them terribly.
2 changes: 1 addition & 1 deletion internal/federation/dereferencing/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (d *Dereferencer) RefreshStatus(
d.dereferenceThread(ctx,
requestUser,
uri,
status,
latest,
statusable,
isNew,
)
Expand Down
31 changes: 28 additions & 3 deletions internal/federation/dereferencing/thread.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,34 @@ func (d *Dereferencer) DereferenceStatusAncestors(ctx context.Context, username
// Check for a returned HTTP code via error.
switch code := gtserror.StatusCode(err); {

// Status codes 404 and 410 incicate the status does not exist anymore.
// Gone (410) is the preferred for deletion, but we accept NotFound too.
case code == http.StatusNotFound || code == http.StatusGone:
// 404 may indicate deletion, but can also
// indicate that we don't have permission to
// view the status (it's followers-only and
// we don't follow, for example).
case code == http.StatusNotFound:
// If this reply is followers-only or stricter,
// we can safely assume the status it replies
// to is also followers only or stricter.
//
// In this case we should leave the inReplyTo
// URI in place for visibility filtering,
// and just return since we can go no further.
if status.Visibility == gtsmodel.VisibilityFollowersOnly ||
status.Visibility == gtsmodel.VisibilityMutualsOnly ||
status.Visibility == gtsmodel.VisibilityDirect {
return nil
}

// If the reply is public or unlisted then
// likely the replied-to status is/was public
// or unlisted and has indeed been deleted,
// fall through to the Gone case to clean up.
fallthrough

// Gone (410) definitely indicates deletion.
// Update the status to remove references to
// the now-gone parent.
case code == http.StatusGone:
l.Trace("status orphaned")
current.InReplyToID = ""
current.InReplyToURI = ""
Expand Down
2 changes: 1 addition & 1 deletion internal/visibility/home_timeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A

// Check parent is deref'd.
if next.InReplyToID == "" {
log.Warnf(ctx, "status not yet deref'd: %s", next.InReplyToURI)
log.Debugf(ctx, "status not (yet) deref'd: %s", next.InReplyToURI)
return false, cache.SentinelError
}

Expand Down
2 changes: 1 addition & 1 deletion internal/visibility/public_timeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (f *Filter) isStatusPublicTimelineable(ctx context.Context, requester *gtsm
// Fetch next parent to lookup.
parentID := parent.InReplyToID
if parentID == "" {
log.Warnf(ctx, "status not yet deref'd: %s", parent.InReplyToURI)
log.Debugf(ctx, "status not (yet) deref'd: %s", parent.InReplyToURI)
return false, cache.SentinelError
}

Expand Down

0 comments on commit 40f9eef

Please sign in to comment.