RELEASE ENG-243 Initial version of capture.wrapHandler #3809
Conversation
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
Ref eng-243 Signed-off-by: Rafael Leite <2132564+leite08@users.noreply.github.com>
…ndler ENG-243 Initial version of capture.wrapHandler
WalkthroughThis change introduces a new Changes
Sequence Diagram(s)Centralized Lambda Handler Error CapturesequenceDiagram
participant AWSLambda
participant Handler
participant capture.wrapHandler
participant Sentry
AWSLambda->>capture.wrapHandler: Invoke wrapped handler
capture.wrapHandler->>Handler: Execute handler logic
alt Handler throws error
capture.wrapHandler->>Sentry: Capture error and context
capture.wrapHandler->>AWSLambda: Rethrow error
else Handler succeeds
capture.wrapHandler->>AWSLambda: Return handler result
end
SQS-to-Converter Lambda (Refactored Flow)sequenceDiagram
participant SQS
participant capture.wrapHandler
participant Handler
participant Sentry
participant Converter
participant Storage
SQS->>capture.wrapHandler: Event
capture.wrapHandler->>Handler: Process event
Handler->>Handler: Extract and set context
Handler->>Storage: Store preprocessed payload
par
Handler->>Handler: Process attachments
Handler->>Storage: Store partitioned payloads
and
Handler->>Converter: Convert FHIR
end
Handler->>Handler: Hydrate bundle (with timeout)
alt Hydration error
Handler->>Sentry: Log warning
end
Handler->>Storage: Store hydrated/normalized bundle
Handler->>Handler: Post-process and send result
Handler->>capture.wrapHandler: Return or throw error
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (38)
packages/lambdas/src/token-auth.ts (1)
19-19: Track and implement migration tocapture.wrapHandler
The added TODO indicates a pending refactor to replaceSentry.AWSLambda.wrapHandlerwithcapture.wrapHandler. Please ensure this task is tracked (e.g., in an issue or backlog) and consider performing the migration now to avoid accumulating technical debt.Would you like me to draft the implementation for wrapping this handler using
capture.wrapHandler()?packages/lambdas/src/patient-import-result.ts (1)
16-16: Track and implement migration tocapture.wrapHandler
The TODO placeholder marks a future refactor to centralize error capturing viacapture.wrapHandler. Ensure this work is tracked and prioritized, and consider migrating this handler in the current scope.Can I help you generate the updated handler signature using
capture.wrapHandler()?packages/lambdas/src/ehr-compute-resource-diff-bundles.ts (1)
23-23: Track and implement migration tocapture.wrapHandler
This TODO signals an upcoming change to replaceSentry.AWSLambda.wrapHandlerwithcapture.wrapHandler. Confirm that this migration is tracked and decide if we should apply it now to maintain consistency across handlers.Let me know if you’d like a diff for updating this export to use
capture.wrapHandler().packages/lambdas/src/ehr-start-resource-diff-bundles.ts (1)
22-22: Track and implement migration tocapture.wrapHandler
The inserted TODO denotes a pending switch fromSentry.AWSLambda.wrapHandlerto the centralizedcapture.wrapHandler. Please ensure this is captured in your release plan and consider completing the migration in this PR to stay consistent.Would you like me to propose the updated handler wrapper implementation?
packages/lambdas/src/healthie-link-patient.ts (1)
21-21: Track and implement migration tocapture.wrapHandler
This TODO comment flags a future refactor to usecapture.wrapHandlerinstead ofSentry.AWSLambda.wrapHandler. Ensure the task is documented and consider applying the change now for uniform error handling.Can I assist by drafting the necessary wrapper migration here?
packages/lambdas/src/hl7-notification-webhook-sender.ts (1)
19-19: Migrate handler tocapture.wrapHandler()
A TODO was added to replaceSentry.AWSLambda.wrapHandlerwithcapture.wrapHandler()for centralized error capturing and context enrichment. Sincecapture.wrapHandleris available, consider performing this migration now to keep consistency across all Lambda handlers.Apply this diff:
-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent): Promise<void> => { +export const handler = capture.wrapHandler(async (event: SQSEvent): Promise<void> => {(Remove the
@sentry/serverlessimport if it’s no longer used)packages/lambdas/src/conversion-result-notifier.ts (1)
16-16: Migrate handler tocapture.wrapHandler()
You’ve marked this handler for future migration fromSentry.AWSLambda.wrapHandlertocapture.wrapHandler(). To standardize error handling and context enrichment immediately, replace the Sentry wrapper now.Apply this diff:
-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { +export const handler = capture.wrapHandler(async (event: SQSEvent) => {(Remove the
@sentry/serverlessimport if it’s no longer needed)packages/lambdas/src/patient-import-parse.ts (1)
17-17: Migrate handler tocapture.wrapHandler()
The TODO indicates a planned switch fromSentry.AWSLambda.wrapHandlertocapture.wrapHandler(). To apply consistent error capturing and avoid drift, implement this change now.Apply this diff:
-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler( +export const handler = capture.wrapHandler((Prune unused
@sentry/serverlessimport if applicable)packages/lambdas/src/ehr-refresh-ehr-bundles.ts (1)
22-22: Migrate handler tocapture.wrapHandler()
You’ve left a TODO to replaceSentry.AWSLambda.wrapHandlerwithcapture.wrapHandler(). To unify your error handling layer and ensure enriched context, convert this handler now.Apply this diff:
-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { +export const handler = capture.wrapHandler(async (event: SQSEvent) => {(Also remove the
@sentry/serverlessimport if it’s redundant)packages/lambdas/src/ehr-sync-patient.ts (1)
21-21: Migrate handler tocapture.wrapHandler()
A TODO marks this handler for migration fromSentry.AWSLambda.wrapHandlertocapture.wrapHandler(). For consistency and centralized context enrichment, update this now.Apply this diff:
-// TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { +export const handler = capture.wrapHandler(async (event: SQSEvent) => {(Remove
@sentry/serverlessimport if unused)packages/lambdas/src/elation-link-patient.ts (1)
21-21: Include JIRA ticket reference in TODO comment.To improve traceability and consistency across handlers, update the TODO to reference ENG-243 and clarify the migration target:
// TODO(ENG-243): migrate to `capture.wrapHandler()` instead of `Sentry.AWSLambda.wrapHandler()`packages/lambdas/src/tester.ts (1)
19-19: Reference the ticket in the TODO comment.For clarity and consistency, annotate the migration task with the JIRA issue and the exact replacement:
// TODO(ENG-243): switch to `capture.wrapHandler()` in place of `Sentry.AWSLambda.wrapHandler()`packages/lambdas/src/scheduled.ts (1)
24-24: Enhance TODO with ticket and expected change.Include the ENG-243 reference and specify the target wrapper function:
// TODO(ENG-243): replace `Sentry.AWSLambda.wrapHandler` with `capture.wrapHandler()` for unified error capturingpackages/lambdas/src/acm-cert-monitor.ts (1)
17-17: Clarify TODO with JIRA and migration details.Update the comment to track the task and desired change:
// TODO(ENG-243): migrate handler to `capture.wrapHandler()` and remove direct Sentry wrappingpackages/lambdas/src/ihe-outbound-document-query.ts (1)
20-20: Enhance TODO comment with ticket and context.To guide future refactoring, adjust the TODO as follows:
// TODO(ENG-243): replace `Sentry.AWSLambda.wrapHandler` with `capture.wrapHandler()` to centralize error contextpackages/lambdas/src/ihe-outbound-patient-discovery.ts (1)
20-20: Suggest standardizing TODO format with JIRA reference.
For better traceability across the codebase, consider updating the comment to:// TODO ENG-243: move to capture.wrapHandler()This makes it clear which ticket drives the change and follows the
TODO:convention.packages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery.ts (1)
14-14: Suggest standardizing TODO format with JIRA reference.
To improve consistency and traceability, please tweak the comment to:// TODO ENG-243: move to capture.wrapHandler()packages/lambdas/src/ihe-gateway-v2-outbound-document-query.ts (1)
14-14: Suggest standardizing TODO format with JIRA reference.
Enhance the comment for clarity and consistency:// TODO ENG-243: move to capture.wrapHandler()packages/lambdas/src/cw-doc-contribution.ts (1)
32-32: Suggest standardizing TODO format with JIRA reference.
For better cross-reference, update to:// TODO ENG-243: move to capture.wrapHandler()packages/lambdas/src/document-uploader.ts (1)
13-13: Suggest standardizing TODO format with JIRA reference.
To maintain consistency and aid tracking, consider:// TODO ENG-243: move to capture.wrapHandler()packages/lambdas/src/patient-import-create.ts (1)
28-28: Ensure migration tocapture.wrapHandler()is tracked
A TODO has been added to migrate this handler to use the new centralized wrapper. Please confirm that there's a corresponding ticket or backlog item to complete this refactor so it doesn’t get forgotten.Would you like me to draft the issue or implement the wrapper now?
packages/lambdas/src/hl7v2-roster.ts (1)
8-8: Ensure migration tocapture.wrapHandler()is tracked
You've added a placeholder to wrap this handler incapture.wrapHandler(). Please verify there's an issue or task scheduled for this migration to avoid leaving it indefinitely as a TODO.Shall I open the ticket or apply the wrapper directly?
packages/lambdas/src/ihe-gateway-v2-inbound-document-retrieval.ts (1)
28-29: Align handler with new capture.wrapHandler pattern
You’ve added a TODO marking this handler for migration, but it’s still a plainexport async function. To centralize error capture and context enrichment, consider wrapping it withcapture.wrapHandlernow.Example diff:
+ import { capture } from "./shared/capture"; - // TODO move to capture.wrapHandler() - export async function handler(event: APIGatewayProxyEventV2) { + export const handler = capture.wrapHandler(async (event: APIGatewayProxyEventV2) => {packages/lambdas/src/fhir-to-bundle.ts (1)
29-30: Consolidate error handling via capture.wrapHandler
You’ve marked the handler for refactoring but left it as a plain async function. Wrap it withcapture.wrapHandlerto leverage the shared error/context logic.- // TODO move to capture.wrapHandler() - export async function handler( + export const handler = capture.wrapHandler(async ((The
captureimport is already present on line 9.)packages/lambdas/src/document-bulk-signer.ts (1)
17-18: Migrate from Sentry to capture.wrapHandler
The handler is still usingSentry.AWSLambda.wrapHandler. Switch tocapture.wrapHandlerfor unified handling and remove the unused Sentry import.- import * as Sentry from "@sentry/serverless"; + // import removed, using capture instead ... - // TODO move to capture.wrapHandler() - export const handler = Sentry.AWSLambda.wrapHandler( + export const handler = capture.wrapHandler(packages/lambdas/src/cw-enhanced-coverage-link-patients.ts (1)
54-55: Standardize with capture.wrapHandler
Rather than wrapping withSentry.AWSLambda.wrapHandler, usecapture.wrapHandlerto centralize your error and context enrichment. Remove the Sentry import.- import * as Sentry from "@sentry/serverless"; + // import removed, using capture instead ... - // TODO move to capture.wrapHandler() - export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { + export const handler = capture.wrapHandler(async (event: SQSEvent) => {packages/lambdas/src/cw-session-management.ts (1)
48-49: Transition handler to capture.wrapHandler
Replace the Sentry wrapper withcapture.wrapHandlerto unify error reporting and remove the Sentry import.- import * as Sentry from "@sentry/serverless"; + // import removed, using capture instead ... - // TODO move to capture.wrapHandler() - export const handler = Sentry.AWSLambda.wrapHandler(async () => { + export const handler = capture.wrapHandler(async () => {packages/lambdas/src/patient-import-query.ts (1)
27-28: Standardize handler wrapping withcapture.wrapHandler
The added TODO correctly marks the upcoming refactor. Once the new wrapper is stable, replace the currentSentry.AWSLambda.wrapHandlercall withcapture.wrapHandlerto unify error capture and context enrichment across all Lambdas.Example diff:
- // TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async function handler(event: SQSEvent) { + // TODO move to capture.wrapHandler() +export const handler = capture.wrapHandler(async function handler(event: SQSEvent) {packages/lambdas/src/ihe-gateway-v2-inbound-patient-discovery.ts (1)
26-27: Align handler export withcapture.wrapHandler
The TODO note is on point. To centralize error handling and context enrichment, wrap the Lambda entrypoint withcapture.wrapHandler(...)instead of exporting a bare async function.Example diff:
- // TODO move to capture.wrapHandler() -export async function handler(event: APIGatewayProxyEventV2) { + // TODO move to capture.wrapHandler() +export const handler = capture.wrapHandler(async (event: APIGatewayProxyEventV2) => {Note: you may need to adjust the return shape to match APIGatewayProxy integration.
packages/lambdas/src/ihe-outbound-document-retrieval.ts (1)
20-21: Switch tocapture.wrapHandlerfor consistency
The TODO is a good prompt. When migrating, replace theSentry.AWSLambda.wrapHandler(call withcapture.wrapHandler(to leverage the new centralized capture logic.Example diff:
- // TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler( + // TODO move to capture.wrapHandler() +export const handler = capture.wrapHandler(packages/lambdas/src/sqs-to-sqs.ts (1)
22-23: Migrate tocapture.wrapHandler
The added comment flags this file for the same wrapper migration. Replace the Sentry wrapper with the newcapture.wrapHandlerto ensure uniform error/context handling.Example diff:
- // TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler(async (event: SQSEvent) => { + // TODO move to capture.wrapHandler() +export const handler = capture.wrapHandler(async (event: SQSEvent) => {packages/lambdas/src/ihe-gateway-v2-outbound-document-retrieval.ts (1)
14-15: Unify withcapture.wrapHandler
Good catch on the TODO. When you perform the migration, swap outSentry.AWSLambda.wrapHandlerforcapture.wrapHandlerto centralize capture logic and metadata enrichment.Example diff:
- // TODO move to capture.wrapHandler() -export const handler = Sentry.AWSLambda.wrapHandler( + // TODO move to capture.wrapHandler() +export const handler = capture.wrapHandler(packages/api/src/command/medical/patient/consolidated-get.ts (2)
98-103: Be careful with very largeJSON.stringifypayloads in logs
JSON.stringify(currentConsolidatedProgress)can easily reach tens / hundreds of KB if the query includes lots of resources, producing verbose CloudWatch logs and throttling the console buffer.
Consider serialising only the relevant keys (e.g.requestId,resources,dateFrom,dateTo) or adding a size guard.- `Patient ${patientId} consolidatedQuery is already 'processing' with params: ${JSON.stringify( - currentConsolidatedProgress - )}, skipping...` + `Patient ${patientId} consolidatedQuery already 'processing'. ` + + `params: ${JSON.stringify(_.pick(currentConsolidatedProgress, ['requestId','resources','dateFrom','dateTo']))}`
299-302: Minor: unify error message & log contextNice rename 👍.
While you’re here, prepend the original error message (errorToString(error)) to make the CloudWatch line self-contained without having to open Sentry.- log(`${msg}: ${JSON.stringify(filters)}`); + log(`${msg}: ${errorToString(error)} | filters=${JSON.stringify(filters)}`);packages/lambdas/src/document-downloader.ts (1)
35-40: Add missing identifiers to Sentry extras for easier triageGood move to
capture.wrapHandler.
You might also want to includelambdaName(already) andnpi/fileInfo.documentUniqueId– those are usually what support / on-call engineers search for.- capture.setExtra({ lambdaName, cxId, orgOid }); + capture.setExtra({ lambdaName, cxId, orgOid, npi, documentUniqueId: document?.uniqueId });packages/lambdas/src/shared/capture.ts (2)
3-8: Import order & tree-shaking
MetriportErroris pulled from@metriport/sharedonly for theinstanceofcheck below, so it’s fine.
HowevererrorToStringis only used insidewrapHandler; consider moving the import next to that function to let bundlers tree-shake it out of other lambdas that don’t callwrapHandler.
90-102: Generic type parameters improve DX for callers ofwrapHandler
AWSLambda.Handlerdefaults to(any, any), so consumers lose compile-time type safety forevent/result. Expose generics so callers can retain their specific shapes:- wrapHandler: (handler: AWSLambda.Handler): AWSLambda.Handler => { + wrapHandler: <E = unknown, C = AWSLambda.Context, R = unknown>( + handler: AWSLambda.Handler<E, R> + ): AWSLambda.Handler<E, R> => {This is backwards-compatible and requires no call-site changes.
packages/lambdas/src/fhir-to-medical-record2.ts (1)
78-81: Tiny string interpolation typoThere is an extra
}afterbucket: ${bucketName}which ends up in the log line.- `dateTo: ${dateTo}, fileName: ${fhirFileName}, bucket: ${bucketName}}` + `dateTo: ${dateTo}, fileName: ${fhirFileName}, bucket: ${bucketName}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
packages/api/src/command/medical/patient/con 8000 solidated-get.ts(2 hunks)packages/core/src/external/aws/lambda.ts(2 hunks)packages/lambdas/src/acm-cert-monitor.ts(1 hunks)packages/lambdas/src/cda-to-visualization.ts(1 hunks)packages/lambdas/src/conversion-result-notifier.ts(1 hunks)packages/lambdas/src/cw-doc-contribution.ts(1 hunks)packages/lambdas/src/cw-enhanced-coverage-link-patients.ts(1 hunks)packages/lambdas/src/cw-session-management.ts(1 hunks)packages/lambdas/src/document-bulk-signer.ts(1 hunks)packages/lambdas/src/document-downloader.ts(1 hunks)packages/lambdas/src/document-uploader.ts(1 hunks)packages/lambdas/src/ehr-compute-resource-diff-bundles.ts(1 hunks)packages/lambdas/src/ehr-refresh-ehr-bundles.ts(1 hunks)packages/lambdas/src/ehr-start-resource-diff-bundles.ts(1 hunks)packages/lambdas/src/ehr-sync-patient.ts(1 hunks)packages/lambdas/src/elation-link-patient.ts(1 hunks)packages/lambdas/src/fhir-to-bundle-count.ts(1 hunks)packages/lambdas/src/fhir-to-bundle.ts(1 hunks)packages/lambdas/src/fhir-to-cda-converter.ts(1 hunks)packages/lambdas/src/fhir-to-medical-record2.ts(2 hunks)packages/lambdas/src/healthie-link-patient.ts(1 hunks)packages/lambdas/src/hl7-notification-webhook-sender.ts(1 hunks)packages/lambdas/src/hl7v2-roster.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-inbound-document-query.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-inbound-document-retrieval.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-inbound-patient-discovery.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-outbound-document-query.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-outbound-document-retrieval.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery-write-to-s3.ts(1 hunks)packages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery.ts(1 hunks)packages/lambdas/src/ihe-outbound-document-query.ts(1 hunks)packages/lambdas/src/ihe-outbound-document-retrieval.ts(1 hunks)packages/lambdas/src/ihe-outbound-patient-discovery.ts(1 hunks)packages/lambdas/src/patient-import-create.ts(1 hunks)packages/lambdas/src/patient-import-parse.ts(1 hunks)packages/lambdas/src/patient-import-query.ts(1 hunks)packages/lambdas/src/patient-import-result.ts(1 hunks)packages/lambdas/src/patient-import-upload-notification.ts(1 hunks)packages/lambdas/src/scheduled.ts(1 hunks)packages/lambdas/src/shared/capture.ts(2 hunks)packages/lambdas/src/sqs-to-converter.ts(2 hunks)packages/lambdas/src/sqs-to-opensearch-xml.ts(1 hunks)packages/lambdas/src/sqs-to-sqs.ts(1 hunks)packages/lambdas/src/tester.ts(1 hunks)packages/lambdas/src/token-auth.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try to use immutable code and avoid sharing state across different functions, objects, and systems - Try...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
packages/lambdas/src/healthie-link-patient.tspackages/lambdas/src/hl7v2-roster.tspackages/lambdas/src/document-uploader.tspackages/lambdas/src/ehr-compute-resource-diff-bundles.tspackages/lambdas/src/ehr-sync-patient.tspackages/lambdas/src/patient-import-parse.tspackages/lambdas/src/hl7-notification-webhook-sender.tspackages/lambdas/src/patient-import-query.tspackages/lambdas/src/fhir-to-bundle-count.tspackages/lambdas/src/acm-cert-monitor.tspackages/lambdas/src/tester.tspackages/lambdas/src/elation-link-patient.tspackages/lambdas/src/fhir-to-cda-converter.tspackages/lambdas/src/conversion-result-notifier.tspackages/lambdas/src/ihe-outbound-patient-discovery.tspackages/lambdas/src/sqs-to-sqs.tspackages/lambdas/src/ihe-outbound-document-query.tspackages/lambdas/src/ehr-refresh-ehr-bundles.tspackages/lambdas/src/fhir-to-bundle.tspackages/lambdas/src/ihe-outbound-document-retrieval.tspackages/lambdas/src/ihe-gateway-v2-inbound-document-retrieval.tspackages/lambdas/src/token-auth.tspackages/lambdas/src/ihe-gateway-v2-inbound-document-query.tspackages/lambdas/src/ehr-start-resource-diff-bundles.tspackages/lambdas/src/cw-enhanced-coverage-link-patients.tspackages/lambdas/src/ihe-gateway-v2-inbound-patient-discovery.tspackages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery-write-to-s3.tspackages/lambdas/src/cw-session-management.tspackages/lambdas/src/patient-import-result.tspackages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery.tspackages/lambdas/src/ihe-gateway-v2-outbound-document-retrieval.tspackages/lambdas/src/document-bulk-signer.tspackages/lambdas/src/cda-to-visualization.tspackages/core/src/external/aws/lambda.tspackages/api/src/command/medical/patient/consolidated-get.tspackages/lambdas/src/patient-import-create.tspackages/lambdas/src/scheduled.tspackages/lambdas/src/sqs-to-opensearch-xml.tspackages/lambdas/src/document-downloader.tspackages/lambdas/src/ihe-gateway-v2-outbound-document-query.tspackages/lambdas/src/patient-import-upload-notification.tspackages/lambdas/src/fhir-to-medical-record2.tspackages/lambdas/src/cw-doc-contribution.tspackages/lambdas/src/shared/capture.tspackages/lambdas/src/sqs-to-converter.ts
🧬 Code Graph Analysis (3)
packages/lambdas/src/document-downloader.ts (1)
packages/lambdas/src/shared/capture.ts (1)
capture(18-103)
packages/lambdas/src/shared/capture.ts (4)
packages/lambdas/src/sqs-to-converter.ts (1)
handler(101-328)packages/lambdas/src/document-downloader.ts (1)
handler(35-79)packages/lambdas/src/fhir-to-medical-record2.ts (1)
handler(63-169)packages/shared/src/index.ts (1)
errorToString(42-42)
packages/lambdas/src/sqs-to-converter.ts (3)
packages/lambdas/src/shared/capture.ts (1)
capture(18-103)packages/core/src/external/cda/process-attachments.ts (1)
processAttachments(64-185)packages/shared/src/index.ts (1)
errorToString(42-42)
🔇 Additional comments (20)
packages/lambdas/src/ihe-outbound-patient-discovery.ts (1)
20-20: Approve placeholder for future migration.
The added// TODO move to capture.wrapHandler()aligns with the overall refactoring plan to centralize error capturing viacapture.wrapHandler(). No functional behavior has changed in this handler.packages/lambdas/src/ihe-gateway-v2-outbound-patient-discovery.ts (1)
14-14: Approve placeholder for future migration.
The// TODO move to capture.wrapHandler()comment correctly marks this handler for the upcoming switch tocapture.wrapHandler(), with no runtime impact.packages/lambdas/src/ihe-gateway-v2-outbound-document-query.ts (1)
14-14: Approve placeholder for future migration.
This added// TODO move to capture.wrapHandler()is in line with the refactoring strategy and has no effect on existing behavior.packages/lambdas/src/cw-doc-contribution.ts (1)
32-32: Approve placeholder for future migration.
The new// TODO move to capture.wrapHandler()correctly flags this handler for migration without altering its functionality.packages/lambdas/src/document-uploader.ts (1)
13-13: Approve placeholder for future migration.
This comment properly marks the handler for later conversion tocapture.wrapHandler()and introduces no behavior changes.packages/lambdas/src/cda-to-visualization.ts (1)
35-35: Appropriate marker for future handler refactoring.The TODO comment aligns with the PR's objective to introduce a centralized error capturing mechanism using
capture.wrapHandler(). This is a preparatory step for future implementation.packages/lambdas/src/sqs-to-opensearch-xml.ts (1)
44-44: Well-placed TODO marker for handler migration.This comment correctly identifies a future opportunity to migrate from manual try/catch error handling to the new centralized
capture.wrapHandler(). This aligns with the PR's goal of standardizing error handling across Lambda functions.packages/lambdas/src/fhir-to-cda-converter.ts (1)
16-16: Appropriate marker for future handler refactoring.The TODO comment correctly identifies this handler as a candidate for migration from
Sentry.AWSLambda.wrapHandlerto the newcapture.wrapHandler()utility. This change is consistent with the PR's objective to standardize error handling.packages/lambdas/src/patient-import-upload-notification.ts (1)
20-20: Well-placed TODO marker for handler migration.This comment correctly identifies a future opportunity to migrate from manual error handling to the new centralized
capture.wrapHandler(). Currently, this handler implements capture context viacapture.setExtra()but still uses manual try/catch blocks for error handling.packages/core/src/external/aws/lambda.ts (1)
5-5: Improved logging with structured logger.The change from using
console.logtoout("getLambdaResultPayload").logaligns with the coding guidelines to avoid direct console logging. This standardized logging approach provides better context and consistency across the codebase.The implementation correctly:
- Imports the required
outfunction from the logging utility- Applies it as the default parameter with appropriate context label
- Maintains the same function signature and behavior
Also applies to: 105-105
packages/lambdas/src/shared/capture.ts (1)
92-100: Scope leakage between warm invocationsThe Sentry Lambda integration resets the scope per invocation, but only if the wrapper is the outer-most one. Because we call
capture.setExtrainside our own wrapper, it’s safe.
Just double-check that other utility functions do not callcapture.setExtraoutside the Lambda-scope (e.g., in async void tasks after the handler resolves) or the data could leak.
No action required, just a heads-up.packages/lambdas/src/fhir-to-medical-record2.ts (1)
63-75: Great adoption ofcapture.wrapHandlerThe new wrapper removes a lot of duplicated try/catch plumbing and makes the handler more readable.
The extra context (lambdaName,cxId,patientId, …) will be invaluable in Sentry.
LGTM!packages/lambdas/src/sqs-to-converter.ts (8)
31-31: Good addition of errorToString for improved error message formattingThe import of
errorToStringfrom the shared package is a good addition that helps standardize error message formatting throughout the codebase.
101-328: Great implementation of wrapHandler for centralized error handlingThe refactoring to use
capture.wrapHandleris a significant improvement that centralizes error capturing and reporting to Sentry. This aligns with the PR objective to automatically callcaptureSetExtraon Lambda handlers.
129-131: Good extraction of message attributes with improved namingThe extraction of message attributes is well done. I notice that you've renamed
serverUrltoconverterUrlin the variable assignment, which improves code clarity by making the purpose of the URL more explicit.
134-134: Good addition of validation for converterUrlAdding validation for the
converterUrlparameter is important for catching configuration issues early in the process.
158-170: Good extraction of attachment processing logicThe extraction of attachment processing into a separate async function improves code modularity and readability. This follows the functional programming principles mentioned in the coding guidelines.
209-226: Good use of Promise.all for concurrent processingUsing
Promise.allto concurrently execute the conversion, attachment processing, and payload storage is a good performance optimization. The reordering to store the preprocessed payload before conversion also makes logical sense.
245-254: Good implementation of timeout for hydrationAdding a timeout race for hydration that rejects after 5 seconds is a great improvement for handling potentially slow hydration processes. This prevents the Lambda from hanging indefinitely on hydration operations.
268-280: Good error handling for hydration failuresThe enhanced error handling for hydration failures is well-implemented. The detailed error message, proper logging with
errorToString, and the decision to continue without hydration rather than failing the entire process shows thoughtful error handling.
Issues:
Dependencies
none
Description
Testing
Check each PR.
Release Plan
masterSummary by CodeRabbit
Refactor
Style
Bug Fixes