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

Don't create unnecessary error objects #1908

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Nov 30, 2023

Creating error objects is not super cheap because it requires the runtime to capture a stack trace. Currently we're creating a lot of error objects every time we handle a response because of this logic.

This logic tries to validate that something is either a DID or a handle by trying to validate one option in a try, and falling back to validating another option in a catch. However, DIDs and handles are mutually exclusive, and we can always tell early which validation function to use. So let's do that so that we don't create throwaway error objects on non-exceptional situations.

Before

I enabled "pause on caught exceptions" and tried to paginate Notifications.

before_throw.mov

After

after_no_throw.mov

Copy link
Collaborator

@dholms dholms left a comment

Choose a reason for hiding this comment

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

yup yup great catch 💪

@dholms dholms merged commit 3c0ef38 into main Nov 30, 2023
2 checks passed
@dholms dholms deleted the dont-throw-if-its-not-needed branch November 30, 2023 23:53
estrattonbailey added a commit that referenced this pull request Dec 5, 2023
…tab-should-show-own-threads

* origin/main: (59 commits)
  Config to start notifications daemon from a specific did (#1922)
  Feature branch: PDS v2 (#1789)
  Cleanup outdated notifications in appview, add daemon for similar tasks (#1893)
  Add flag for running db migrations on appview (#1913)
  Do not generate notifs when post violates threadgate (#1901)
  Version packages (#1909)
  Additional @atproto/api 0.6.24 changeset (#1912)
  Fix snapshots for list items (#1911)
  Attach record URI to listItemView (#1758)
  helpers for rkey and tid syntax; validate rkey at record creation time (#1738)
  harden datetime verification (#1702)
  fix(debug): properly type debugCatch wrapper result (#1817)
  style(xrpc-server): avoid un-neccessary "if" statement (#1826)
  perf(bsky): avoid re-creating auth functions on every request (#1822)
  Don't create unnecessary error objects (#1908)
  fix(pds): include aspectRatio on read-sticky posts (#1824)
  Handle missing creator on lists and feed generators (#1906)
  ✨ Expose labels attached with legacy actions when events are queried and fix email event builder (#1905)
  Evented architecture for moderation system (#1617)
  Put canReply state on post viewer state instead of thread viewer state (#1882)
  ...
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.

None yet

2 participants