fix(vps): complete resolve HttpApi cutover (Step 6)#162
Merged
Conversation
PR #158 ported resolve to HttpApiBuilder.group and correctly deleted the old Hono trio on its own branch (migration/6-resolve-group, stacked on migration/6-profile-group), but that branch never got merged forward into migration/effect-http-api -- the integration branch moved directly from #157's merge to the step 6b docs/client-swap commits. Result: the old Hono resolve.routes.ts/.index.ts/.handlers.ts stayed on disk and app.ts kept routing /api/resolve to them, while apps/www's resolve hook (#161) already called the typed client -- pointed at a route that also still worked, but via dead code shadowing the real port. Cherry-picks cad53ec (the original port commit) onto the current tip: wires ResolveHandlersLive into http/routes.ts, removes the /api/resolve Hono mount from app.ts, deletes the superseded Hono files.
guidefari
commented
Jul 11, 2026
Comment on lines
+8
to
+16
| const dieOnDatabaseError = <A, E, R>(effect: Effect.Effect<A, E | DatabaseErrorTag, R>) => | ||
| effect.pipe( | ||
| Effect.tapErrorTag('DatabaseError', (cause) => | ||
| Effect.logError('[resolve] database operation failed', cause) | ||
| ), | ||
| Effect.catchTag('DatabaseError', (cause) => Effect.die(cause)) | ||
| ) | ||
|
|
||
| type DatabaseErrorTag = { readonly _tag: 'DatabaseError' } |
Owner
Author
There was a problem hiding this comment.
dieOnDatabaseError feels like a duplicate?
Owner
Author
There was a problem hiding this comment.
Good catch, fixed: extracted to apps/vps/src/http/handler-utils.ts as a tag -> wrapper factory (dieOnDatabaseError(tag) => wrapper), so this file and search/music/profile handlers all share one definition instead of copy-pasting it. See 114ae54.
guidefari
added a commit
that referenced
this pull request
Jul 11, 2026
Review on #162 flagged dieOnDatabaseError as a likely duplicate -- correct: search, music, profile, and this PR's admin handlers each had an identical copy differing only in the log-tag prefix string. Extracted to apps/vps/src/http/handler-utils.ts as a tag -> wrapper factory, with each handler file partially applying its own tag once at module scope so call sites stay unchanged (dieOnDatabaseError(effect)).
Addresses review comment: dieOnDatabaseError was an identical copy-paste across search/music/profile/resolve handlers, differing only in the log-tag prefix string. Extracted to apps/vps/src/http/handler-utils.ts as a tag -> wrapper factory; this PR updates resolve.handlers.ts to use it. The other three handler files' duplicate copies are updated on migration/6-admin-group (PR #163), which also introduces the shared helper -- once both branches merge into migration/effect-http-api, all four groups share one definition.
guidefari
added a commit
that referenced
this pull request
Jul 11, 2026
Review on #162 flagged dieOnDatabaseError as a likely duplicate -- correct: search, music, profile, and this PR's admin handlers each had an identical copy differing only in the log-tag prefix string. Extracted to apps/vps/src/http/handler-utils.ts as a tag -> wrapper factory, with each handler file partially applying its own tag once at module scope so call sites stay unchanged (dieOnDatabaseError(effect)).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
resolve(step 6) was ported toHttpApiBuilder.groupin #158, on branchmigration/6-resolve-groupstacked onmigration/6-profile-group. Both theport and the old Hono file deletion happened correctly in that commit
(
cad53ec9) -- butmigration/6-profile-group's tip (with #158 alreadystacked on it) never got merged forward into
migration/effect-http-api.The integration branch went straight from #157's merge to the step 6b docs
and client-swap commits instead, so #158 is absent from
migration/effect-http-api's history even though GitHub shows it as merged.Net effect:
apps/vps/src/app.tskept mounting the old Honoroutes/resolve/*trio at/api/resolve, andpackages/api/src/resolve.tsapps/vps/src/http/resolve.handlers.tsnever existed on this branch atall. Meanwhile feat(www): swap resolve consumer to typed Effect client (step 6b) #161 already swapped
apps/www'suseResolveSlughook tocall
client.resolve.resolveSlug(...)against that same path -- it workedbecause the old Hono route was still live, not because the Effect port was
serving it.
What to look for
cad53ec9(PR feat(vps): port resolve to HttpApiBuilder.group (Step 6) #158's single commit) onto thecurrent tip, applied clean with no conflicts -- not a re-port. Diff should
match feat(vps): port resolve to HttpApiBuilder.group (Step 6) #158's original diff:
resolve.handlers.ts+resolve.tsschemaadded,
routes/resolve/{resolve.routes,resolve.index,resolve.handlers}.tsdeleted,
app.tsmount removed,ResolveHandlersLivewired intohttp/routes.ts'sApiLive.routes/resolveremain anywhere inapps/vps/srcorapps/www/srcafter the delete.apps/www's resolve hookalready targets the typed client (feat(www): swap resolve consumer to typed Effect client (step 6b) #161); this PR only fixes which
server-side handler actually answers that path.
Test evidence
No UI-visible behavior change (confirmed by grep:
apps/www/src/lib/http.tsalready calls
client.resolve.resolveSlugfrom #161, unchanged by this PR),so no new screenshot -- the existing #158 evidence (show page rendering
bannerImageUrl/tags/hosts[].username) already covers this handler'scorrectness; this PR only fixes routing.
bun run typecheck(vps): clean.vitest run --config vitest.unit.config.ts(vps): the newresolve (HttpApiBuilder group, Step 6)blackbox case fails with the samepre-existing DB-connectivity error (
500instead of404/200) affecting7 other tests in this file on unmodified
migration/effect-http-api(health, music-artists, search, profile) -- verified by running the same
suite on
origin/migration/effect-http-apibefore this change: identicalfailure count and shape (
Failed queryerrors frommusic_artists/audiotables, i.e. the local testcontainers DB isn't reachable in this session).
Not introduced by this change; not fixed by this change either, since it's
an environment issue, not a code issue.