[bugfix] Ensure InReplyToID set properly, update dereference ancestors func (#1921)

internal/federation/dereferencing/dereferencer.go

@@ -66,6 +66,12 @@ type Dereferencer interface {
 	// This is a more optimized form of manually enqueueing .UpdateStatus() to the federation worker, since it only enqueues update if necessary.
 	RefreshStatusAsync(ctx context.Context, requestUser string, status *gtsmodel.Status, apubStatus ap.Statusable, force bool)
 
+	// DereferenceStatusAncestors iterates upwards from the given status, using InReplyToURI, to ensure that as many parent statuses as possible are dereferenced.
+	DereferenceStatusAncestors(ctx context.Context, requestUser string, status *gtsmodel.Status) error
+
+	// DereferenceStatusDescendents iterates downwards from the given status, using its replies, to ensure that as many children statuses as possible are dereferenced.
+	DereferenceStatusDescendants(ctx context.Context, requestUser string, statusIRI *url.URL, parent ap.Statusable) error
+
 	GetRemoteInstance(ctx context.Context, username string, remoteInstanceURI *url.URL) (*gtsmodel.Instance, error)
 
 	DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error

internal/federation/dereferencing/status.go

@@ -104,7 +104,7 @@ func (d *deref) getStatusByURI(ctx context.Context, requestUser string, uri *url
 	}
 
 	if status == nil {
-		// Ensure that this is isn't a search for a local status.
+		// Ensure that this isn't a search for a local status.
 		if uri.Host == config.GetHost() || uri.Host == config.GetAccountDomain() {
 			return nil, nil, gtserror.SetUnretrievable(err) // this will be db.ErrNoEntries
 		}
@@ -149,7 +149,7 @@ func (d *deref) getStatusByURI(ctx context.Context, requestUser string, uri *url
 // RefreshStatus: implements Dereferencer{}.RefreshStatus().
 func (d *deref) RefreshStatus(ctx context.Context, requestUser string, status *gtsmodel.Status, apubStatus ap.Statusable, force bool) (*gtsmodel.Status, ap.Statusable, error) {
 	// Check whether needs update.
-	if statusUpToDate(status) {
+	if !force && statusUpToDate(status) {
 		return status, nil, nil
 	}
 
@@ -205,8 +205,16 @@ func (d *deref) RefreshStatusAsync(ctx context.Context, requestUser string, stat
 	})
 }
 
-// enrichStatus will enrich the given status, whether a new barebones model, or existing model from the database. It handles necessary dereferencing etc.
-func (d *deref) enrichStatus(ctx context.Context, requestUser string, uri *url.URL, status *gtsmodel.Status, apubStatus ap.Statusable) (*gtsmodel.Status, ap.Statusable, error) {
+// enrichStatus will enrich the given status, whether a new
+// barebones model, or existing model from the database.
+// It handles necessary dereferencing, database updates, etc.
+func (d *deref) enrichStatus(
+	ctx context.Context,
+	requestUser string,
+	uri *url.URL,
+	status *gtsmodel.Status,
+	apubStatus ap.Statusable,
+) (*gtsmodel.Status, ap.Statusable, error) {
 	// Pre-fetch a transport for requesting username, used by later dereferencing.
 	tsport, err := d.transportController.NewTransportForUsername(ctx, requestUser)
 	if err != nil {
@@ -217,7 +225,8 @@ func (d *deref) enrichStatus(ctx context.Context, requestUser string, uri *url.U
 	if blocked, err := d.state.DB.IsDomainBlocked(ctx, uri.Host); err != nil {
 		return nil, nil, gtserror.Newf("error checking blocked domain: %w", err)
 	} else if blocked {
-		return nil, nil, gtserror.Newf("%s is blocked", uri.Host)
+		err = gtserror.Newf("%s is blocked", uri.Host)
+		return nil, nil, gtserror.SetUnretrievable(err)
 	}
 
 	if apubStatus == nil {

internal/federation/dereferencing/thread.go

@@ -19,6 +19,8 @@ package dereferencing
 
 import (
 	"context"
+	"errors"
+	"net/http"
 	"net/url"
 
 	"codeberg.org/gruf/go-kv"
@@ -26,96 +28,184 @@ import (
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
-	"github.com/superseriousbusiness/gotosocial/internal/uris"
 )
 
 // maxIter defines how many iterations of descendants or
 // ancesters we are willing to follow before returning error.
 const maxIter = 1000
 
-// dereferenceThread will dereference statuses both above and below the given status in a thread, it returns no error and is intended to be called asychronously.
 func (d *deref) dereferenceThread(ctx context.Context, username string, statusIRI *url.URL, status *gtsmodel.Status, statusable ap.Statusable) {
 	// Ensure that ancestors have been fully dereferenced
-	if err := d.dereferenceStatusAncestors(ctx, username, status); err != nil {
-		log.Error(ctx, err) // log entry and error will include caller prefixes
+	if err := d.DereferenceStatusAncestors(ctx, username, status); err != nil {
+		log.Error(ctx, err)
 	}
 
 	// Ensure that descendants have been fully dereferenced
-	if err := d.dereferenceStatusDescendants(ctx, username, statusIRI, statusable); err != nil {
-		log.Error(ctx, err) // log entry and error will include caller prefixes
+	if err := d.DereferenceStatusDescendants(ctx, username, statusIRI, statusable); err != nil {
+		log.Error(ctx, err)
 	}
 }
 
-// dereferenceAncestors has the goal of reaching the oldest ancestor of a given status, and stashing all statuses along the way.
-func (d *deref) dereferenceStatusAncestors(ctx context.Context, username string, status *gtsmodel.Status) error {
-	// Take ref to original
-	ogIRI := status.URI
-
-	// Start log entry with fields
-	l := log.WithContext(ctx).
-		WithFields(kv.Fields{
-			{"username", username},
-			{"statusIRI", ogIRI},
-		}...)
-
-	// Log function start
-	l.Trace("beginning")
+func (d *deref) DereferenceStatusAncestors(
+	ctx context.Context,
+	username string,
+	status *gtsmodel.Status,
+) error {
+	// Mark given status as the one
+	// we're currently working on.
+	var current = status
 
 	for i := 0; i < maxIter; i++ {
-		if status.InReplyToURI == "" {
-			// status doesn't reply to anything
+		if current.InReplyToURI == "" {
+			// Status has no parent, we've
+			// reached the top of the chain.
 			return nil
 		}
 
-		// Parse this status's replied IRI
-		replyIRI, err := url.Parse(status.InReplyToURI)
-		if err != nil {
-			return gtserror.Newf("invalid status InReplyToURI %q: %w", status.InReplyToURI, err)
+		l := log.
+			WithContext(ctx).
+			WithFields(kv.Fields{
+				{"username", username},
+				{"originalStatusIRI", status.URI},
+				{"currentStatusURI", current.URI},
+				{"currentInReplyToURI", current.InReplyToURI},
+			}...)
+
+		if current.InReplyToID != "" {
+			// We already have an InReplyToID set. This means
+			// the status's parent has, at some point, been
+			// inserted into the database, either because it
+			// is a status from our instance, or a status from
+			// remote that we've dereferenced before, or found
+			// out about in some other way.
+			//
+			// Working on this assumption, check if the parent
+			// status exists, either as a copy pinned on the
+			// current status, or in the database.
+
+			if current.InReplyTo != nil {
+				// We have the parent already, and the child
+				// doesn't need to be updated; keep iterating
+				// from this parent upwards.
+				current = current.InReplyTo
+				continue
+			}
+
+			// Parent isn't pinned to this status (yet), see
+			// if we can get it from the db (we should be
+			// able to, since it has an ID already).
+			parent, err := d.state.DB.GetStatusByID(
+				gtscontext.SetBarebones(ctx),
+				current.InReplyToID,
+			)
+			if err != nil && !errors.Is(err, db.ErrNoEntries) {
+				// Real db error, stop.
+				return gtserror.Newf("db error getting status %s: %w", current.InReplyToID, err)
+			}
+
+			if parent != nil {
+				// We got the parent from the db, and the child
+				// doesn't need to be updated; keep iterating
+				// from this parent upwards.
+				current.InReplyTo = parent
+				current = parent
+				continue
+			}
+
+			// If we arrive here, we know this child *did* have
+			// a parent at some point, but it no longer exists in
+			// the database, presumably because it's been deleted
+			// by another action.
+			//
+			// TODO: clean this up in a nightly task.
+			l.Warnf("current status has been orphaned (parent %s no longer exists in database)", current.InReplyToID)
+			return nil // Cannot iterate further.
 		}
 
-		if replyIRI.Host == config.GetHost() {
-			l.Tracef("following local status ancestors: %s", status.InReplyToURI)
+		// If we reach this point, we know the status has
+		// an InReplyToURI set, but it doesn't yet have an
+		// InReplyToID, which means that the parent status
+		// has not yet been dereferenced.
+		inReplyToURI, err := url.Parse(current.InReplyToURI)
+		if err != nil || inReplyToURI == nil {
+			// Parent URI is not something we can handle.
+			l.Debug("current status has been orphaned (invalid InReplyToURI)")
+			return nil //nolint:nilerr
+		}
 
-			// This is our status, extract ID from path
-			_, id, err := uris.ParseStatusesPath(replyIRI)
-			if err != nil {
-				return gtserror.Newf("invalid local status IRI %q: %w", status.InReplyToURI, err)
+		// Parent URI is valid, try to get it.
+		// getStatusByURI guards against the following conditions:
+		//
+		//   - remote domain is blocked (will return unretrievable)
+		//   - domain is local (will try to return something, or
+		//     return unretrievable).
+		parent, _, err := d.getStatusByURI(ctx, username, inReplyToURI)
+		if err == nil {
+			// We successfully fetched the parent.
+			// Update current status with new info.
+			current.InReplyToID = parent.ID
+			current.InReplyToAccountID = parent.AccountID
+			if err := d.state.DB.UpdateStatus(
+				ctx, current,
+				"in_reply_to_id",
+				"in_reply_to_account_id",
+			); err != nil {
+				return gtserror.Newf("db error updating status %s: %w", current.ID, err)
 			}
 
-			// Fetch this status from the database
-			localStatus, err := d.state.DB.GetStatusByID(ctx, id)
-			if err != nil {
-				return gtserror.Newf("error fetching local status %q: %w", id, err)
+			// Mark parent as next status to
+			// work on, and keep iterating.
+			current = parent
+			continue
+		}
+
+		// We could not fetch the parent, check if we can do anything
+		// useful with the error. For example, HTTP status code returned
+		// from remote may indicate that the parent has been deleted.
+		switch code := gtserror.StatusCode(err); {
+		case code == http.StatusGone || code == http.StatusNotFound:
+			// 410 means the status has definitely been deleted.
+			// 404 means the status has *probably* been deleted.
+			// Update this status to reflect that, then bail.
+			l.Debugf("current status has been orphaned (call to parent returned code %d)", code)
+
+			current.InReplyToURI = ""
+			if err := d.state.DB.UpdateStatus(
+				ctx, current,
+				"in_reply_to_uri",
+			); err != nil {
+				return gtserror.Newf("db error updating status %s: %w", current.ID, err)
 			}
+			return nil
 
-			// Set the fetched status
-			status = localStatus
+		case code != 0:
+			// We had a code, but not one indicating deletion,
+			// log the code but don't return error or update the
+			// status; we can try again later.
+			l.Warnf("cannot dereference parent (%q)", err)
+			return nil
 
-		} else {
-			l.Tracef("following remote status ancestors: %s", status.InReplyToURI)
+		case gtserror.Unretrievable(err):
+			// Not retrievable for some other reason, so just
+			// bail; we can try again later if necessary.
+			l.Debugf("parent unretrievable (%q)", err)
+			return nil
 
-			// Fetch the remote status found at this IRI
-			remoteStatus, _, err := d.getStatusByURI(
-				ctx,
-				username,
-				replyIRI,
-			)
-			if err != nil {
-				return gtserror.Newf("error fetching remote status %q: %w", status.InReplyToURI, err)
-			}
-
-			// Set the fetched status
-			status = remoteStatus
+		default:
+			// Some other error that stops us in our tracks.
+			return gtserror.Newf("error dereferencing parent %s: %w", current.InReplyToURI, err)
 		}
 	}
 
-	return gtserror.Newf("reached %d ancestor iterations for %q", maxIter, ogIRI)
+	return gtserror.Newf("reached %d ancestor iterations for %q", maxIter, status.URI)
 }
 
-func (d *deref) dereferenceStatusDescendants(ctx context.Context, username string, statusIRI *url.URL, parent ap.Statusable) error {
+func (d *deref) DereferenceStatusDescendants(ctx context.Context, username string, statusIRI *url.URL, parent ap.Statusable) error {
 	// Take ref to original
 	ogIRI := statusIRI
 
@@ -256,9 +346,17 @@ stackLoop:
 				}
 
 				// Dereference the remote status and store in the database.
+				// getStatusByURI guards against the following conditions:
+				//
+				//   - remote domain is blocked (will return unretrievable)
+				//   - domain is local (will try to return something, or
+				//     return unretrievable).
 				_, statusable, err := d.getStatusByURI(ctx, username, itemIRI)
 				if err != nil {
-					l.Errorf("error dereferencing remote status %s: %v", itemIRI, err)
+					if !gtserror.Unretrievable(err) {
+						l.Errorf("error dereferencing remote status %s: %v", itemIRI, err)
+					}
+
 					continue itemLoop
 				}
 

internal/processing/fromfederator.go

@@ -107,53 +107,32 @@ func (p *Processor) ProcessFromFederator(ctx context.Context, federatorMsg messa
 	return nil
 }
 
-// processCreateStatusFromFederator handles Activity Create and Object Note
+// processCreateStatusFromFederator handles Activity Create and Object Note.
 func (p *Processor) processCreateStatusFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error {
-	// check for either an IRI that we still need to dereference, OR an already dereferenced
-	// and converted status pinned to the message.
+	// Check the federatorMsg for either an already
+	// dereferenced and converted status pinned to
+	// the message, or an AP IRI that we need to deref.
 	var (
 		status *gtsmodel.Status
 		err    error
 	)
 
 	if federatorMsg.GTSModel != nil {
-		var ok bool
-
-		// there's a gts model already pinned to the message, it should be a status
-		if status, ok = federatorMsg.GTSModel.(*gtsmodel.Status); !ok {
-			return gtserror.New("Note was not parseable as *gtsmodel.Status")
-		}
-
-		// Since this was a create originating AP object
-		// statusable may have been set on message (no problem if not).
-		statusable, _ := federatorMsg.APObjectModel.(ap.Statusable)
-
-		// Call refresh on status to deref if necessary etc.
-		status, _, err = p.federator.RefreshStatus(ctx,
-			federatorMsg.ReceivingAccount.Username,
-			status,
-			statusable,
-			false,
-		)
-		if err != nil {
-			return err
-		}
+		// Model is set, use that.
+		status, err = p.statusFromGTSModel(ctx, federatorMsg)
 	} else {
-		// no model pinned, we need to dereference based on the IRI
-		if federatorMsg.APIri == nil {
-			return gtserror.New("status was not pinned to federatorMsg, and neither was an IRI for us to dereference")
-		}
+		// Model is not set, use IRI.
+		status, err = p.statusFromAPIRI(ctx, federatorMsg)
+	}
 
-		status, _, err = p.federator.GetStatusByURI(ctx, federatorMsg.ReceivingAccount.Username, federatorMsg.APIri)
-		if err != nil {
-			return err
-		}
+	if err != nil {
+		return gtserror.Newf("error extracting status from federatorMsg: %w", err)
 	}
 
 	if status.Account == nil || status.Account.IsRemote() {
 		// Either no account attached yet, or a remote account.
 		// Both situations we need to parse account URI to fetch it.
-		remoteAccURI, err := url.Parse(status.AccountURI)
+		accountURI, err := url.Parse(status.AccountURI)
 		if err != nil {
 			return err
 		}
@@ -161,13 +140,22 @@ func (p *Processor) processCreateStatusFromFederator(ctx context.Context, federa
 		// Ensure that account for this status has been deref'd.
 		status.Account, _, err = p.federator.GetAccountByURI(ctx,
 			federatorMsg.ReceivingAccount.Username,
-			remoteAccURI,
+			accountURI,
 		)
 		if err != nil {
 			return err
 		}
 	}
 
+	// Ensure status ancestors dereferenced. We need at least the
+	// immediate parent (if present) to ascertain timelineability.
+	if err := p.federator.DereferenceStatusAncestors(ctx,
+		federatorMsg.ReceivingAccount.Username,
+		status,
+	); err != nil {
+		return err
+	}
+
 	if status.InReplyToID != "" {
 		// Interaction counts changed on the replied status;
 		// uncache the prepared version from all timelines.
@@ -181,6 +169,58 @@ func (p *Processor) processCreateStatusFromFederator(ctx context.Context, federa
 	return nil
 }
 
+func (p *Processor) statusFromGTSModel(ctx context.Context, federatorMsg messages.FromFederator) (*gtsmodel.Status, error) {
+	// There should be a status pinned to the federatorMsg
+	// (we've already checked to ensure this is not nil).
+	status, ok := federatorMsg.GTSModel.(*gtsmodel.Status)
+	if !ok {
+		err := gtserror.New("Note was not parseable as *gtsmodel.Status")
+		return nil, err
+	}
+
+	// AP statusable representation may have also
+	// been set on message (no problem if not).
+	statusable, _ := federatorMsg.APObjectModel.(ap.Statusable)
+
+	// Call refresh on status to update
+	// it (deref remote) if necessary.
+	var err error
+	status, _, err = p.federator.RefreshStatus(
+		ctx,
+		federatorMsg.ReceivingAccount.Username,
+		status,
+		statusable,
+		false,
+	)
+	if err != nil {
+		return nil, gtserror.Newf("%w", err)
+	}
+
+	return status, nil
+}
+
+func (p *Processor) statusFromAPIRI(ctx context.Context, federatorMsg messages.FromFederator) (*gtsmodel.Status, error) {
+	// There should be a status IRI pinned to
+	// the federatorMsg for us to dereference.
+	if federatorMsg.APIri == nil {
+		err := gtserror.New("status was not pinned to federatorMsg, and neither was an IRI for us to dereference")
+		return nil, err
+	}
+
+	// Get the status + ensure we have
+	// the most up-to-date version.
+	status, _, err := p.federator.GetStatusByURI(
+		ctx,
+		federatorMsg.ReceivingAccount.Username,
+		federatorMsg.APIri,
+	)
+	if err != nil {
+		return nil, gtserror.Newf("%w", err)
+	}
+
+	return status, nil
+}
+
 // processCreateFaveFromFederator handles Activity Create with Object Like.
 func (p *Processor) processCreateFaveFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error {
 	statusFave, ok := federatorMsg.GTSModel.(*gtsmodel.StatusFave)
@@ -278,11 +318,21 @@ func (p *Processor) processCreateAnnounceFromFederator(ctx context.Context, fede
 	}
 	status.ID = statusID
 
-	// Store, timeline, and notify.
+	// Store the boost wrapper status.
 	if err := p.state.DB.PutStatus(ctx, status); err != nil {
 		return gtserror.Newf("db error inserting status: %w", err)
 	}
 
+	// Ensure boosted status ancestors dereferenced. We need at least
+	// the immediate parent (if present) to ascertain timelineability.
+	if err := p.federator.DereferenceStatusAncestors(ctx,
+		federatorMsg.ReceivingAccount.Username,
+		status.BoostOf,
+	); err != nil {
+		return err
+	}
+
+	// Timeline and notify the announce.
 	if err := p.timelineAndNotifyStatus(ctx, status); err != nil {
 		return gtserror.Newf("error timelining status: %w", err)
 	}

internal/typeutils/astointernal.go

@@ -245,134 +245,171 @@ func (c *converter) extractAttachments(i ap.WithAttachment) []*gtsmodel.MediaAtt
 }
 
 func (c *converter) ASStatusToStatus(ctx context.Context, statusable ap.Statusable) (*gtsmodel.Status, error) {
-	status := &gtsmodel.Status{}
+	status := new(gtsmodel.Status)
 
-	// uri at which this status is reachable
-	uriProp := statusable.GetJSONLDId()
-	if uriProp == nil || !uriProp.IsIRI() {
-		return nil, errors.New("no id property found, or id was not an iri")
+	// status.URI
+	//
+	// ActivityPub ID/URI of this status.
+	idProp := statusable.GetJSONLDId()
+	if idProp == nil || !idProp.IsIRI() {
+		return nil, gtserror.New("no id property found, or id was not an iri")
 	}
-	status.URI = uriProp.GetIRI().String()
+	status.URI = idProp.GetIRI().String()
 
 	l := log.WithContext(ctx).
 		WithField("statusURI", status.URI)
 
-	// web url for viewing this status
+	// status.URL
+	//
+	// Web URL of this status (optional).
 	if statusURL, err := ap.ExtractURL(statusable); err == nil {
 		status.URL = statusURL.String()
 	} else {
-		// if no URL was set, just take the URI
-		status.URL = status.URI
+		status.URL = status.URI // Fall back to the URI.
 	}
 
-	// the html-formatted content of this status
+	// status.Content
+	//
+	// The (html-formatted) content of this status.
 	status.Content = ap.ExtractContent(statusable)
 
-	// attachments to dereference and fetch later on (we don't do that here)
+	// status.Attachments
+	//
+	// Media attachments for later dereferencing.
 	status.Attachments = c.extractAttachments(statusable)
 
-	// hashtags to dereference later on
+	// status.Hashtags
+	//
+	// Hashtags for later dereferencing.
 	if hashtags, err := ap.ExtractHashtags(statusable); err != nil {
-		l.Infof("ASStatusToStatus: error extracting status hashtags: %s", err)
+		l.Infof("error extracting hashtags: %q", err)
 	} else {
 		status.Tags = hashtags
 	}
 
-	// emojis to dereference and fetch later on
+	// status.Emojis
+	//
+	// Custom emojis for later dereferencing.
 	if emojis, err := ap.ExtractEmojis(statusable); err != nil {
-		l.Infof("ASStatusToStatus: error extracting status emojis: %s", err)
+		l.Infof("error extracting emojis: %q", err)
 	} else {
 		status.Emojis = emojis
 	}
 
-	// mentions to dereference later on
+	// status.Mentions
+	//
+	// Mentions of other accounts for later dereferencing.
 	if mentions, err := ap.ExtractMentions(statusable); err != nil {
-		l.Infof("ASStatusToStatus: error extracting status mentions: %s", err)
+		l.Infof("error extracting mentions: %q", err)
 	} else {
 		status.Mentions = mentions
 	}
 
-	// cw string for this status
-	// prefer Summary, fall back to Name
+	// status.ContentWarning
+	//
+	// Topic or content warning for this status;
+	// prefer Summary, fall back to Name.
 	if summary := ap.ExtractSummary(statusable); summary != "" {
 		status.ContentWarning = summary
 	} else {
 		status.ContentWarning = ap.ExtractName(statusable)
 	}
 
-	// when was this status created?
+	// status.Published
+	//
+	// Publication time of this status. Thanks to
+	// db defaults, will fall back to now if not set.
 	published, err := ap.ExtractPublished(statusable)
 	if err != nil {
-		l.Infof("ASStatusToStatus: error extracting status published: %s", err)
+		l.Infof("error extracting published: %q", err)
 	} else {
 		status.CreatedAt = published
 		status.UpdatedAt = published
 	}
 
-	// which account posted this status?
-	// if we don't know the account yet we can dereference it later
+	// status.AccountURI
+	// status.AccountID
+	// status.Account
+	//
+	// Account that created the status. Assume we have
+	// this in the db by the time this function is called,
+	// error if we don't.
 	attributedTo, err := ap.ExtractAttributedToURI(statusable)
 	if err != nil {
-		return nil, errors.New("ASStatusToStatus: attributedTo was empty")
+		return nil, gtserror.Newf("%w", err)
 	}
-	status.AccountURI = attributedTo.String()
+	accountURI := attributedTo.String()
 
-	statusOwner, err := c.db.GetAccountByURI(ctx, attributedTo.String())
+	account, err := c.db.GetAccountByURI(ctx, accountURI)
 	if err != nil {
-		return nil, fmt.Errorf("ASStatusToStatus: couldn't get status owner from db: %s", err)
+		err = gtserror.Newf("db error getting status author account %s: %w", accountURI, err)
+		return nil, err
 	}
-	status.AccountID = statusOwner.ID
-	status.AccountURI = statusOwner.URI
-	status.Account = statusOwner
+	status.AccountURI = accountURI
+	status.AccountID = account.ID
+	status.Account = account
 
-	// check if there's a post that this is a reply to
-	inReplyToURI := ap.ExtractInReplyToURI(statusable)
-	if inReplyToURI != nil {
-		// something is set so we can at least set this field on the
-		// status and dereference using this later if we need to
-		status.InReplyToURI = inReplyToURI.String()
+	// status.InReplyToURI
+	// status.InReplyToID
+	// status.InReplyTo
+	// status.InReplyToAccountID
+	// status.InReplyToAccount
+	//
+	// Status that this status replies to, if applicable.
+	// If we don't have this status in the database, we
+	// just set the URI and assume we can deref it later.
+	if uri := ap.ExtractInReplyToURI(statusable); uri != nil {
+		inReplyToURI := uri.String()
+		status.InReplyToURI = inReplyToURI
 
-		// now we can check if we have the replied-to status in our db already
-		if inReplyToStatus, err := c.db.GetStatusByURI(ctx, inReplyToURI.String()); err == nil {
-			// we have the status in our database already
-			// so we can set these fields here and now...
-			status.InReplyToID = inReplyToStatus.ID
-			status.InReplyToAccountID = inReplyToStatus.AccountID
-			status.InReplyTo = inReplyToStatus
-			if status.InReplyToAccount == nil {
-				if inReplyToAccount, err := c.db.GetAccountByID(ctx, inReplyToStatus.AccountID); err == nil {
-					status.InReplyToAccount = inReplyToAccount
-				}
-			}
+		// Check if we already have the replied-to status.
+		inReplyTo, err := c.db.GetStatusByURI(ctx, inReplyToURI)
+		if err != nil && !errors.Is(err, db.ErrNoEntries) {
+			// Real database error.
+			err = gtserror.Newf("db error getting replied-to status %s: %w", inReplyToURI, err)
+			return nil, err
+		}
+
+		if inReplyTo != nil {
+			// We have it in the DB! Set
+			// appropriate fields here and now.
+			status.InReplyToID = inReplyTo.ID
+			status.InReplyTo = inReplyTo
+			status.InReplyToAccountID = inReplyTo.AccountID
+			status.InReplyToAccount = inReplyTo.Account
 		}
 	}
 
-	// visibility entry for this status
-	visibility, err := ap.ExtractVisibility(statusable, status.Account.FollowersURI)
+	// status.Visibility
+	visibility, err := ap.ExtractVisibility(
+		statusable,
+		status.Account.FollowersURI,
+	)
 	if err != nil {
-		return nil, fmt.Errorf("ASStatusToStatus: error extracting visibility: %s", err)
+		err = gtserror.Newf("error extracting visibility: %w", err)
+		return nil, err
 	}
 	status.Visibility = visibility
 
-	// advanced visibility for this status
-	// TODO: a lot of work to be done here -- a new type needs to be created for this in go-fed/activity using ASTOOL
-	// for now we just set everything to true
-	federated := true
-	boostable := true
-	replyable := true
-	likeable := true
+	// Advanced visibility toggles for this status.
+	//
+	// TODO: a lot of work to be done here -- a new type
+	// needs to be created for this in go-fed/activity.
+	// Until this is implemented, assume all true.
+	var trueBool = func() *bool { b := true; return &b }
+	status.Federated = trueBool()
+	status.Boostable = trueBool()
+	status.Replyable = trueBool()
+	status.Likeable = trueBool()
 
-	status.Federated = &federated
-	status.Boostable = &boostable
-	status.Replyable = &replyable
-	status.Likeable = &likeable
-
-	// sensitive
-	sensitive := ap.ExtractSensitive(statusable)
-	status.Sensitive = &sensitive
+	// status.Sensitive
+	status.Sensitive = func() *bool {
+		s := ap.ExtractSensitive(statusable)
+		return &s
+	}()
 
 	// language
-	// we might be able to extract this from the contentMap field
+	// TODO: we might be able to extract this from the contentMap field
 
 	// ActivityStreamsType
 	status.ActivityStreamsType = statusable.GetTypeName()