feat: expose domain dns records via api#259
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughReplaces hard-coded DNS rows in the domain settings UI with dynamic rendering from domainQuery.data?.dnsRecords and updates component prop types to use enriched domain response types (RouterOutputs/DomainResponse/DomainWithDnsRecords). Adds DomainDnsRecordSchema and extends DomainSchema with verificationError, lastCheckedTime, and dnsRecords. Implements domain-service helpers (parseDomainStatus, buildDnsRecords, withDnsRecords) and updates server resolvers/public-api to delegate domain fetching/enrichment to that service (including a new GET /v1/domains/{id} public route). Updates OpenAPI/schema, docs, TypeScript SDK, and Python SDK to expose dnsRecords and new domain-related endpoints/types. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying usesend with
|
| Latest commit: |
839e9b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2333cfbf.usesend.pages.dev |
| Branch Preview URL: | https://codex-fix-api-domain-creatio.usesend.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (4)
131-139: Ensure stable, unique keys for DNS rowstype-name can collide across records (e.g., multiple TXT with same name). Include value or index to guarantee uniqueness.
-const key = `${record.type}-${record.name}`; +const key = `${record.type}-${record.name}-${record.value}`;
185-197: Add rollback on mutation error for click trackingCurrently optimistic UI flips the switch but doesn’t revert on error.
function handleClickTrackingChange() { setClickTracking(!clickTracking); updateDomain.mutate( { id: domain.id, clickTracking: !clickTracking }, { onSuccess: () => { utils.domain.invalidate(); toast.success("Click tracking updated"); }, + onError: (err) => { + setClickTracking((v) => !v); + toast.error(err?.message ?? "Failed to update click tracking"); + }, }, ); }
198-209: Add rollback on mutation error for open trackingMirror error handling for openTracking.
function handleOpenTrackingChange() { setOpenTracking(!openTracking); updateDomain.mutate( { id: domain.id, openTracking: !openTracking }, { onSuccess: () => { utils.domain.invalidate(); toast.success("Open tracking updated"); }, + onError: (err) => { + setOpenTracking((v) => !v); + toast.error(err?.message ?? "Failed to update open tracking"); + }, }, ); }
228-231: Brand consistency: “useSend”Copy says “Unsend”; align with “useSend”.
- Unsend adds a tracking pixel to every email you send. This allows you + useSend adds a tracking pixel to every email you send. This allows youapps/web/src/server/service/domain-service.ts (3)
235-239: Robust DMARC presence check (join TXT chunks, avoid partial access)resolveTxt returns string[][]. Using [0][0] is brittle. Join chunks and check any record exists.
-const _dmarcRecord = await getDmarcRecord(tldts.getDomain(domain.name)!); -const dmarcRecord = _dmarcRecord?.[0]?.[0]; +const orgDomain = tldts.getDomain(domain.name); +const _dmarcRecord = orgDomain ? await getDmarcRecord(orgDomain) : null; +const dmarcRecord = _dmarcRecord?.map(parts => parts.join("")).find(Boolean);And use Boolean(dmarcRecord) where applicable (dmarcAdded, etc.).
Also applies to: 248-256
215-224: Normalize getDomain return shape consistentlyWhen not verifying you return withDnsRecords(domain) without verificationError/lastCheckedTime normalization, but during verifying you include them. Prefer consistent shape to simplify consumers (TRPC/public API).
Option: always compute normalized fields (verificationError?: string | null, lastCheckedTime?: string | null) and include them (null when absent).
Also applies to: 258-278, 281-282
44-75: MX row status uses SPF statusIf you intend independent status per record, consider mapping:
- DKIM TXT → dkimStatus
- SPF TXT → spfStatus
- MX → Mail From status (same as spfStatus today), but verify with SES semantics.
No change required if intentional; flagging for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx(5 hunks)apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsx(1 hunks)apps/web/src/lib/zod/domain-schema.ts(2 hunks)apps/web/src/server/api/routers/domain.ts(2 hunks)apps/web/src/server/public-api/api/domains/create-domain.ts(0 hunks)apps/web/src/server/public-api/api/domains/get-domains.ts(2 hunks)apps/web/src/server/service/domain-service.ts(4 hunks)apps/web/src/types/domain.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/server/public-api/api/domains/create-domain.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsxapps/web/src/lib/zod/domain-schema.tsapps/web/src/server/api/routers/domain.tsapps/web/src/types/domain.tsapps/web/src/server/service/domain-service.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsxapps/web/src/server/public-api/api/domains/get-domains.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript with 2-space indentation and semicolons (enforced by Prettier)
ESLint must pass with zero warnings using @usesend/eslint-config
Do not use dynamic imports (avoid import() and dynamic loading)
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsxapps/web/src/lib/zod/domain-schema.tsapps/web/src/server/api/routers/domain.tsapps/web/src/types/domain.tsapps/web/src/server/service/domain-service.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsxapps/web/src/server/public-api/api/domains/get-domains.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code and docs with Prettier 3
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsxapps/web/src/lib/zod/domain-schema.tsapps/web/src/server/api/routers/domain.tsapps/web/src/types/domain.tsapps/web/src/server/service/domain-service.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsxapps/web/src/server/public-api/api/domains/get-domains.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsxapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the "/" alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC for API calls unless explicitly instructed otherwise
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsxapps/web/src/lib/zod/domain-schema.tsapps/web/src/server/api/routers/domain.tsapps/web/src/types/domain.tsapps/web/src/server/service/domain-service.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsxapps/web/src/server/public-api/api/domains/get-domains.ts
🧬 Code graph analysis (5)
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsx (1)
apps/web/src/types/domain.ts (1)
DomainWithDnsRecords(13-17)
apps/web/src/server/api/routers/domain.ts (1)
apps/web/src/server/service/domain-service.ts (1)
getDomains(318-333)
apps/web/src/types/domain.ts (1)
packages/python-sdk/usesend/types.py (1)
Domain(26-42)
apps/web/src/server/service/domain-service.ts (2)
packages/python-sdk/usesend/types.py (1)
Domain(26-42)apps/web/src/types/domain.ts (1)
DomainDnsRecord(3-11)
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (2)
apps/web/src/server/api/root.ts (1)
AppRouter(40-40)packages/ui/src/text-with-copy.tsx (1)
TextWithCopyButton(6-41)
🔇 Additional comments (6)
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (2)
36-43: Verify Next.js/React support for use(params) in client pageUsing use(params: Promise<...>) in a client component relies on React 19 + Next 15 pattern. If you’re on Next 14 or not using React 19, switch to useParams().
Example alternative:
-export default function DomainItemPage({ params }: { params: Promise<{ domainId: string }> }) { - const { domainId } = use(params); +import { useParams } from "next/navigation"; +export default function DomainItemPage() { + const { domainId } = useParams<{ domainId: string }>();
4-4: No Prisma enums in client component
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx no longer importsDomainStatusfrom@prisma/client; no changes needed.Likely an incorrect or invalid review comment.
apps/web/src/server/service/domain-service.ts (1)
33-76: Likely incorrect DNS host composition for subdomains; prefer FQDNsUsing subdomainSuffix = .${domain.subdomain} yields names like mail.news for a domain “news.example.com”, which is not an FQDN and can mislead users (and may be wrong relative to the hosted zone). Recommend emitting FQDNs:
- MX/TXT (SPF) at mail.
- DKIM at ._domainkey.
- DMARC at _dmarc.
Apply:
-function buildDnsRecords(domain: Domain): DomainDnsRecord[] { - const subdomainSuffix = domain.subdomain ? `.${domain.subdomain}` : ""; - const mailDomain = `mail${subdomainSuffix}`; - const dkimSelector = domain.dkimSelector ?? "usesend"; +function buildDnsRecords(domain: Domain): DomainDnsRecord[] { + const fullDomain = domain.name; // e.g., "news.example.com" or "example.com" + const orgDomain = tldts.getDomain(domain.name) ?? domain.name; // e.g., "example.com" + const mailHost = `mail.${fullDomain}`; + const dkimSelector = domain.dkimSelector ?? "usesend"; const spfStatus = parseDomainStatus(domain.spfDetails); const dkimStatus = parseDomainStatus(domain.dkimStatus); const dmarcStatus = domain.dmarcAdded ? DomainStatus.SUCCESS : DomainStatus.NOT_STARTED; return [ { type: "MX", - name: mailDomain, + name: mailHost, value: `feedback-smtp.${domain.region}.amazonses.com`, ttl: "Auto", priority: "10", status: spfStatus, }, { type: "TXT", - name: `${dkimSelector}._domainkey${subdomainSuffix}`, + name: `${dkimSelector}._domainkey.${fullDomain}`, value: `p=${domain.publicKey}`, ttl: "Auto", status: dkimStatus, }, { type: "TXT", - name: mailDomain, + name: mailHost, value: "v=spf1 include:amazonses.com ~all", ttl: "Auto", status: spfStatus, }, { type: "TXT", - name: "_dmarc", + name: `_dmarc.${orgDomain}`, value: "v=DMARC1; p=none;", ttl: "Auto", status: dmarcStatus, recommended: true, }, ]; }Please confirm whether your UI expects FQDNs or relative names; adjust “name” accordingly to match customer DNS provider UX.
apps/web/src/app/(dashboard)/domains/[domainId]/send-test-mail.tsx (1)
8-13: Align props with DNS-enriched domain shapeUsing
DomainWithDnsRecordskeeps this component in sync with the expanded domain payload from the service layer while preserving access to the base Prisma fields such asid. Looks good.apps/web/src/server/api/routers/domain.ts (1)
45-46: Centralize domain fetching through the serviceRouting the list query through
getDomainsensures the TRPC response picks up the DNS records and normalized metadata produced bywithDnsRecords, matching the new contract everywhere we expose domains. 👍apps/web/src/server/public-api/api/domains/get-domains.ts (1)
26-28: Reuse service logic for public API domainsDelegating to
getDomainsServicegives the public API the same DNS-enriched shape and optional domain scoping that the internal routes already enjoy, avoiding divergence in the serialization path. Nice consolidation.
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/python-sdk/usesend/domains.py (1)
1-38: Remove the unusednoqaand move the type-only importRuff flags
# noqa: E402as unused (RUF100). ImportUseSendinside aTYPE_CHECKINGblock instead and drop the suppression so linting passes. Based on static analysis hints.-from typing import Optional, Tuple, List +from typing import TYPE_CHECKING, Optional, Tuple, List @@ -class Domains: +if TYPE_CHECKING: + from .usesend import UseSend + + +class Domains: @@ - def __init__(self, usesend: "UseSend") -> None: + def __init__(self, usesend: "UseSend") -> None: self.usesend = usesend @@ -from .usesend import UseSend # noqa: E402 pylint: disable=wrong-import-position
🧹 Nitpick comments (1)
apps/web/src/server/service/domain-service.ts (1)
13-27: Consider using a Map for better performanceThe
Setlookup for domain status validation is functionally correct, but aMapor direct enum comparison would be more explicit about the intended behavior.-const DOMAIN_STATUS_VALUES = new Set(Object.values(DomainStatus)); - -function parseDomainStatus(status?: string | null): DomainStatus { - if (!status) { - return DomainStatus.NOT_STARTED; - } - - const normalized = status.toUpperCase(); - - if (DOMAIN_STATUS_VALUES.has(normalized as DomainStatus)) { - return normalized as DomainStatus; - } - - return DomainStatus.NOT_STARTED; -} +function parseDomainStatus(status?: string | null): DomainStatus { + if (!status) { + return DomainStatus.NOT_STARTED; + } + + const normalized = status.toUpperCase(); + + if (Object.values(DomainStatus).includes(normalized as DomainStatus)) { + return normalized as DomainStatus; + } + + return DomainStatus.NOT_STARTED; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/docs/api-reference/domains/get-domain.mdx(1 hunks)apps/docs/api-reference/domains/list-domains.mdx(1 hunks)apps/docs/api-reference/openapi.json(12 hunks)apps/docs/docs.json(4 hunks)apps/web/src/server/api/routers/domain.ts(4 hunks)apps/web/src/server/public-api/api/domains/get-domain.ts(1 hunks)apps/web/src/server/public-api/index.ts(2 hunks)apps/web/src/server/service/domain-service.ts(5 hunks)package.json(2 hunks)packages/python-sdk/pyproject.toml(1 hunks)packages/python-sdk/usesend/__init__.py(1 hunks)packages/python-sdk/usesend/domains.py(1 hunks)packages/python-sdk/usesend/types.py(3 hunks)packages/python-sdk/usesend/usesend.py(2 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/domain.ts(2 hunks)packages/sdk/src/email.ts(1 hunks)packages/sdk/types/schema.d.ts(8 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/sdk/package.json
- packages/python-sdk/pyproject.toml
- packages/sdk/src/email.ts
- apps/docs/api-reference/domains/list-domains.mdx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/public-api/api/domains/get-domain.tspackages/sdk/src/domain.tsapps/web/src/server/public-api/index.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/api/routers/domain.tspackages/sdk/types/schema.d.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript with 2-space indentation and semicolons (enforced by Prettier)
ESLint must pass with zero warnings using @usesend/eslint-config
Do not use dynamic imports (avoid import() and dynamic loading)
Files:
apps/web/src/server/public-api/api/domains/get-domain.tspackages/sdk/src/domain.tsapps/web/src/server/public-api/index.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/api/routers/domain.tspackages/sdk/types/schema.d.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code and docs with Prettier 3
Files:
apps/web/src/server/public-api/api/domains/get-domain.tspackages/sdk/src/domain.tsapps/web/src/server/public-api/index.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/api/routers/domain.tspackages/sdk/types/schema.d.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the "/" alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC for API calls unless explicitly instructed otherwise
Files:
apps/web/src/server/public-api/api/domains/get-domain.tsapps/web/src/server/public-api/index.tsapps/web/src/server/service/domain-service.tsapps/web/src/server/api/routers/domain.ts
🧬 Code graph analysis (8)
apps/web/src/server/public-api/api/domains/get-domain.ts (4)
apps/web/src/lib/zod/domain-schema.ts (1)
DomainSchema(34-56)apps/web/src/server/service/domain-service.ts (1)
getDomain(210-279)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
packages/python-sdk/usesend/usesend.py (2)
packages/python-sdk/usesend/domains.py (1)
Domains(15-35)packages/sdk/src/domain.ts (1)
Domains(40-73)
packages/python-sdk/usesend/domains.py (2)
packages/python-sdk/usesend/types.py (5)
APIError(335-337)Domain(41-60)DomainCreate(66-68)DomainCreateResponse(71-90)DomainVerifyResponse(93-94)packages/python-sdk/usesend/usesend.py (4)
get(109-110)post(106-107)put(112-113)UseSend(34-121)
packages/sdk/src/domain.ts (2)
packages/sdk/types/index.ts (1)
ErrorResponse(1-4)packages/sdk/types/schema.d.ts (1)
paths(6-907)
apps/web/src/server/public-api/index.ts (1)
apps/web/src/server/service/domain-service.ts (1)
getDomain(210-279)
packages/python-sdk/usesend/__init__.py (1)
packages/python-sdk/usesend/domains.py (1)
Domains(15-35)
apps/web/src/server/service/domain-service.ts (2)
packages/python-sdk/usesend/types.py (1)
Domain(41-60)apps/web/src/types/domain.ts (1)
DomainDnsRecord(3-11)
apps/web/src/server/api/routers/domain.ts (2)
apps/web/src/server/service/domain-service.ts (2)
getDomains(315-330)getDomain(210-279)apps/web/src/server/api/trpc.ts (1)
domainProcedure(165-176)
🪛 Ruff (0.13.1)
packages/python-sdk/usesend/usesend.py
127-127: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
packages/python-sdk/usesend/domains.py
37-37: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
packages/python-sdk/usesend/__init__.py
7-7: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
package.json (1)
34-34: LGTM: Standard Mintlify dependency updateThe bump from 4.0.510 to 4.2.128 appears to be a standard maintenance update. Updates to the most recent version of the Mintlify CLI typically include improvements and bug fixes for documentation tooling.
apps/docs/api-reference/domains/get-domain.mdx (1)
2-2: LGTM: OpenAPI path correctly updated for single domain retrievalThe path change from "GET /v1/domains" to "GET /v1/domains/{id}" accurately reflects the new endpoint for retrieving a specific domain by ID, aligning with the broader PR changes that introduce domain-by-id functionality.
apps/web/src/server/service/domain-service.ts (5)
29-72: LGTM: DNS record construction logic is well-structuredThe
buildDnsRecordsfunction correctly constructs DNS records with proper subdomain handling, status parsing, and includes all required record types (MX, TXT for SPF/DKIM, and DMARC).
74-81: LGTM: Clean domain augmentation patternThe
withDnsRecordshelper follows a clean pattern for augmenting domains with DNS record data while preserving type safety.
202-207: LGTM: Proper initialization with DNS recordsThe
createDomainfunction correctly initializes the new domain status fields and applies DNS record augmentation to the returned domain.
210-279: LGTM: Comprehensive domain verification handlingThe
getDomainfunction properly handles both static and verifying domains, with appropriate normalization and augmentation. The verification flow correctly updates domain status and includes all necessary fields in the response.
315-330: LGTM: Consistent domain augmentationThe
getDomainsfunction maintains consistency by applying the same DNS record augmentation to all returned domains, with proper filtering support.apps/web/src/server/api/routers/domain.ts (1)
14-14: LGTM: Clean migration to domain service layerThe changes properly delegate domain retrieval to the domain service layer, ensuring consistent data augmentation with DNS records across all domain endpoints. The import addition and function calls are correctly implemented.
Also applies to: 44-46, 48-50
apps/docs/api-reference/openapi.json (5)
105-171: LGTM: Comprehensive DNS records schema additionThe DNS records schema is well-defined with proper field types, enums, and required fields. The schema correctly includes:
- DNS record types (MX, TXT)
- All necessary fields (type, name, value, ttl, priority, status, recommended)
- Proper nullable handling for priority field
- Status enum matching domain verification states
Also applies to: 291-357, 540-606
27-27: LGTM: Accurate endpoint descriptionsThe endpoint descriptions have been updated to properly reflect their functionality:
- GET /v1/domains now describes retrieving domains accessible by API key
- PUT /v1/domains/{id}/verify correctly describes domain verification
- GET /v1/domains/{id} properly describes single domain retrieval
Also applies to: 392-392, 464-464
409-445: LGTM: Proper error response documentationThe addition of 403 and 404 error responses for the domain verification endpoint provides clear API contract documentation for error handling scenarios.
180-181: LGTM: DNS records properly marked as requiredThe addition of
dnsRecordsto the required fields array ensures API consumers can expect this field to always be present in domain responses.Also applies to: 366-367, 615-616
748-750: LGTM: Email status enum extensionThe addition of "CANCELLED" and "SUPPRESSED" to the email status enums properly extends the email event tracking capabilities.
Also applies to: 1012-1014
| let enriched; | ||
| try { | ||
| enriched = await getDomainService(id, team.id); | ||
| } catch (e) { | ||
| throw new UnsendApiError({ | ||
| code: "NOT_FOUND", | ||
| message: "Domain not found", | ||
| }); | ||
| } | ||
|
|
||
| return c.json(enriched); |
There was a problem hiding this comment.
Preserve non-404 failures from the domain service
Catching every exception from getDomainService and returning NOT_FOUND hides legitimate server failures (e.g., AWS SES/network/DB issues inside withDnsRecords). This misleads clients, complicates debugging, and masks outages as missing resources. Only translate the explicit “Domain not found” sentinel to 404; anything else should surface as an internal error.
- let enriched;
- try {
- enriched = await getDomainService(id, team.id);
- } catch (e) {
- throw new UnsendApiError({
- code: "NOT_FOUND",
- message: "Domain not found",
- });
- }
-
- return c.json(enriched);
+ try {
+ const enriched = await getDomainService(id, team.id);
+ return c.json(enriched);
+ } catch (error) {
+ if (error instanceof Error && error.message === "Domain not found") {
+ throw new UnsendApiError({
+ code: "NOT_FOUND",
+ message: "Domain not found",
+ });
+ }
+
+ throw new UnsendApiError({
+ code: "INTERNAL_SERVER_ERROR",
+ message: "Failed to retrieve domain",
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let enriched; | |
| try { | |
| enriched = await getDomainService(id, team.id); | |
| } catch (e) { | |
| throw new UnsendApiError({ | |
| code: "NOT_FOUND", | |
| message: "Domain not found", | |
| }); | |
| } | |
| return c.json(enriched); | |
| try { | |
| const enriched = await getDomainService(id, team.id); | |
| return c.json(enriched); | |
| } catch (error) { | |
| if (error instanceof Error && error.message === "Domain not found") { | |
| throw new UnsendApiError({ | |
| code: "NOT_FOUND", | |
| message: "Domain not found", | |
| }); | |
| } | |
| throw new UnsendApiError({ | |
| code: "INTERNAL_SERVER_ERROR", | |
| message: "Failed to retrieve domain", | |
| }); | |
| } |
| from . import types | ||
|
|
||
| __all__ = ["UseSend", "UseSendHTTPError", "types"] | ||
| __all__ = ["UseSend", "UseSendHTTPError", "types", "Domains"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep __all__ sorted to satisfy Ruff.
Ruff (RUF022) enforces a sorted __all__. Reordering the exports alphabetically will clear the lint failure.
(Based on static analysis hints)
-__all__ = ["UseSend", "UseSendHTTPError", "types", "Domains"]
+__all__ = ["Domains", "UseSend", "UseSendHTTPError", "types"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| __all__ = ["UseSend", "UseSendHTTPError", "types", "Domains"] | |
| __all__ = ["Domains", "UseSend", "UseSendHTTPError", "types"] |
🧰 Tools
🪛 Ruff (0.13.1)
7-7: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
In packages/python-sdk/usesend/__init__.py around line 7, the __all__ list is
not alphabetically sorted (RUF022); reorder the exported names in
ASCII/alphabetical order so that the list is sorted (e.g., put "Domains" first,
then "UseSend", then "UseSendHTTPError", then "types") to satisfy Ruff.
| DNSRecordType = Literal['MX', 'TXT'] | ||
|
|
||
|
|
||
| class DNSRecord(TypedDict, total=False): | ||
| type: DNSRecordType | ||
| name: str | ||
| value: str | ||
| ttl: str | ||
| priority: Optional[str] | ||
| status: DomainStatus | ||
| recommended: Optional[bool] | ||
|
|
||
|
|
||
| DNSRecords = List[DNSRecord] |
There was a problem hiding this comment.
Include CNAME in DNSRecordType.
buildDnsRecords surfaces DKIM CNAME entries from SES, so the API will return records with type: "CNAME". Constraining the Literal to "MX"/"TXT" makes the SDK types inaccurate and will trip type-checkers once those records show up. Please add "CNAME" to the Literal.
-DNSRecordType = Literal['MX', 'TXT']
+DNSRecordType = Literal['CNAME', 'MX', 'TXT']📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DNSRecordType = Literal['MX', 'TXT'] | |
| class DNSRecord(TypedDict, total=False): | |
| type: DNSRecordType | |
| name: str | |
| value: str | |
| ttl: str | |
| priority: Optional[str] | |
| status: DomainStatus | |
| recommended: Optional[bool] | |
| DNSRecords = List[DNSRecord] | |
| DNSRecordType = Literal['CNAME', 'MX', 'TXT'] | |
| class DNSRecord(TypedDict, total=False): | |
| type: DNSRecordType | |
| name: str | |
| value: str | |
| ttl: str | |
| priority: Optional[str] | |
| status: DomainStatus | |
| recommended: Optional[bool] | |
| DNSRecords = List[DNSRecord] |
🤖 Prompt for AI Agents
In packages/python-sdk/usesend/types.py around lines 25 to 38, the DNSRecordType
Literal currently only allows 'MX' and 'TXT' but SES can return DKIM CNAME
entries; update the DNSRecordType definition to include 'CNAME' so the TypedDict
accurately represents API responses and avoids type-checker failures.
| # Import here to avoid circular dependency during type checking | ||
| from .emails import Emails # noqa: E402 pylint: disable=wrong-import-position | ||
| from .contacts import Contacts # noqa: E402 pylint: disable=wrong-import-position | ||
| from .domains import Domains # type: ignore # noqa: E402 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Drop the unused noqa directive.
Ruff (RUF100) reports # noqa: E402 as unused because that rule isn’t enabled; trimming it keeps the lint run clean.
(Based on static analysis hints)
-from .domains import Domains # type: ignore # noqa: E402
+from .domains import Domains # type: ignore📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from .domains import Domains # type: ignore # noqa: E402 | |
| from .domains import Domains # type: ignore |
🧰 Tools
🪛 Ruff (0.13.1)
127-127: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In packages/python-sdk/usesend/usesend.py around line 127, the import line
includes an unused comment "# noqa: E402"; remove that directive so the line
reads simply "from .domains import Domains" to satisfy Ruff (RUF100) and keep
lint output clean.
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68d43c1206c88329b5e02c69fbae2067
Summary by CodeRabbit
New Features
Improvements
SDK & Docs