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

[bugfix] Don't return Account or Status if new and dereferencing failed, other small fixes #2563

Merged
merged 11 commits into from
Jan 26, 2024

Conversation

tsmethurst
Copy link
Contributor

Description

If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.

If this is a documentation change, please briefly describe what you've changed and why.

This pull request fixes up some of our account and status dereferencing logic in the following ways:

  • Don't return a near-zero Account or Status when it's new and a call to the remote instance returned a 4xx status code. Log + return an error instead. Previously, returning a partial account or status which hadn't yet been inserted properly in the database was causing all sorts of problems, since it would be put in the cache with zero values all over it. See [bug] [federation] honk (tedu) #2527 and [bug] Panic in AccountToAPIAccountPublic when looking up account from Catodon #2514.
  • Derive published time for accounts using the published property. This should allow the actual remote creation time of the account to be stored, instead of just the time that the GtS instance first saw the account, which was misleading in that it was making some accounts look way newer than they actually are.
  • Refactor some of the webfinger response parsing logic a bit to reuse existing regular expressions, add a new regular expression for the user web path (/@username).

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@tsmethurst tsmethurst changed the title [bugfix] Don't return Account or Status if new and dereferencing failed [bugfix] Don't return Account or Status if new and dereferencing failed, other small fixes Jan 22, 2024
@tsmethurst tsmethurst marked this pull request as draft January 22, 2024 11:27
@tsmethurst tsmethurst merged commit e3052e8 into main Jan 26, 2024
3 checks passed
@tsmethurst tsmethurst deleted the account_deref_webfinger_tidying branch January 26, 2024 13:17
tsmethurst added a commit that referenced this pull request Feb 14, 2024
…ed, other small fixes (#2563)

* tidy up account, status, webfingering logic a wee bit

* go fmt

* invert published check

* alter resp initialization

* get Published from account in typeutils

* don't instantiate error for no darn good reason

* shadow err

* don't repeat error codes in wrapped errors

* don't wrap error unnecessarily
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