Skip to content

Add searchable record and search 🔍#231

Open
Drache93 wants to merge 14 commits intomainfrom
search
Open

Add searchable record and search 🔍#231
Drache93 wants to merge 14 commits intomainfrom
search

Conversation

@Drache93
Copy link

Records are added with simhashed tokens for later searching

@Drache93 Drache93 requested a review from mafintosh December 31, 2025 18:46
@Drache93 Drache93 force-pushed the search branch 2 times, most recently from 0fe27d4 to c710340 Compare December 31, 2025 18:53
Records are added with simhashed tokens for later searching
@Drache93 Drache93 marked this pull request as ready for review December 31, 2025 18:53
}

async onsearchablerecordput(req) {
if (!req.target || !this.searchableRecords) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!req.target || !this.searchableRecords) return
if (!req.target || !this.searchableRecords) return

no req.reply(null) before return - intentional?

Copy link
Author

@Drache93 Drache93 Jan 12, 2026

Choose a reason for hiding this comment

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

Yes, was intentional based on approach i saw on the other functions. req.reply(null) was added to onsearch so when we run https://github.com/holepunchto/hyperdht/pull/231/changes#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R192 there's no issues

Any downside?

Copy link
Contributor

Choose a reason for hiding this comment

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

no sure but if there's no req.reply called I'd have thought it could lead to a leak maybe, underlying stream wouldn't be ended?

Copy link
Author

Choose a reason for hiding this comment

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

Checked with @mafintosh , all good since same in the other functions.

chetasr and others added 2 commits January 15, 2026 17:42
* updates + fixes

* remove debug

* rollback to package
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