Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
backend/src/api/public/v1/members/work-experiences/verifyMemberWorkExperience.ts
Outdated
Show resolved
Hide resolved
backend/src/api/public/v1/members/identities/verifyMemberIdentity.ts
Outdated
Show resolved
Hide resolved
backend/src/api/public/v1/members/work-experiences/verifyMemberWorkExperience.ts
Outdated
Show resolved
Hide resolved
joanagmaia
left a comment
joanagmaia
left a comment
There was a problem hiding this comment.
@skwowet only 1 endpoint missing at this point which is to get the project affiliations with the associated maintainers roles services/apps/data_sink_worker/src/service/member.service.ts.
We can add this after, non-blocking for the PR
| await qx.tx(async (tx) => { | ||
| await cleanSoftDeletedMemberOrganization(tx, memberId, data.organizationId, data) | ||
|
|
||
| newMemberOrgId = await createMemberOrganization(tx, memberId, memberOrgData) |
There was a problem hiding this comment.
If createMemberOrganization fails to insert undefined is returned because on conflict do nothing returning id means that if there is a conflict do nothing but if there is no conflict insert and return id. Meaning that in case of a conflict we don't get existing id from the db so we return undefined... Meaning that end result could be NotFoundError('Work experience not found after creation') even though we could create work experience for existing org.
There was a problem hiding this comment.
Updated it so that the query now uses a CTE with UNION ALL to fall back to selecting the existing row on conflict.
backend/src/api/public/v1/members/identities/verifyMemberIdentity.ts
Outdated
Show resolved
Hide resolved
| if (unmerge) { | ||
| const { preview, result } = unmerge | ||
|
|
||
| await captureApiChange( |
There was a problem hiding this comment.
If captureApiChange or startMemberUnmergeWorkflow fails we get 500 back even though the db changes are done in the above code and the tx is commited.
There was a problem hiding this comment.
Wrapped post-commit side effects (audit log, cache invalidation, workflow start) in individual try/catch blocks so failures in these don't return 500 when the DB transaction already committed successfully.
Also added support for slack notification so that we are alerted about it. Even if the DB changes go through we should still be able to know that these async processes failed to run.
| import type { ApiRequest, Auth0TokenPayload } from '@/types/api' | ||
| import type { Auth0TokenPayload } from '@/types/api' | ||
|
|
||
| function resolveActor(req: Request, _res: Response, next: NextFunction): void { |
There was a problem hiding this comment.
Not actually in this file but the errorHandler.ts that handles error never logs anything. Unknown errors become generic "Internal server error" with the original error discarded. No logs, no alerts — complete observability blind spot for all public API failures. We should do logging there and/or do a slack notification to alert for critical errors.
There was a problem hiding this comment.
Added req.log.error logging and a Slack alert via sendSlackNotification for unhandled errors, matching the pattern used in apiResponseHandler.
backend/src/api/public/v1/members/work-experiences/createMemberWorkExperience.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| ] | ||
| : []), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Unawaited promise may cause unhandled rejection crash
Low Severity
sendSlackNotification returns a Promise that is neither awaited nor has a .catch() handler. If the Slack call fails (e.g., network timeout), the rejected promise becomes an unhandled rejection, which can crash the Node.js process depending on the runtime configuration.
3 participants
Note
Cursor Bugbot is generating a summary for commit a1b3652. Configure here.