Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update proxy data type for response handler input #3030

Merged
merged 28 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7d6ea12
feat: update proxy data type for response handler input
utsabc Jan 25, 2024
b1327eb
feat: update proxy v1 test cases
utsabc Jan 25, 2024
30c4eca
Merge branch 'develop' into fix.network-handlers
utsabc Jan 25, 2024
9dd8625
feat: update proxy tests for cm360
utsabc Jan 29, 2024
650911e
fix: typo
utsabc Jan 29, 2024
84b6a5d
Update test/integrations/destinations/campaign_manager/dataDelivery/b…
utsabc Jan 29, 2024
686f524
Update test/integrations/destinations/campaign_manager/dataDelivery/b…
utsabc Jan 29, 2024
76e0284
fix: api contract for v1 proxy
utsabc Feb 1, 2024
93947db
fix: api contract for v1 proxy (#3049)
utsabc Feb 2, 2024
d2e65f4
chore: clean up zod type
Feb 5, 2024
7ce0a66
chore: update testutils
Feb 5, 2024
99f5cb2
chore: update V0 proxy request type and zod schema
Feb 5, 2024
0504ffa
feat: update proxy tests for cm360 (#3039)
utsabc Feb 7, 2024
bd6af6b
Merge branch 'develop' into fix.network-handlers
utsabc Feb 7, 2024
014bf5e
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Feb 8, 2024
b57a926
Merge branch 'fix.network-handlers' of github.com:rudderlabs/rudder-t…
Feb 8, 2024
325433b
feat: adding zod validations (#3066)
utsabc Feb 8, 2024
689b0cd
chore: update delivery test cases for criteo audience
ItsSudip Feb 8, 2024
9e04774
Revert "chore: update delivery test cases for criteo audience"
ItsSudip Feb 8, 2024
7114f9b
chore: add type def for proxy v1 test
Feb 9, 2024
33d4d62
chore: fix generateMetdata func
Feb 9, 2024
455dce7
chore: criteo audience update proxy test (#3068)
ItsSudip Feb 13, 2024
c7133b3
chore: enable batch response schema check (#3083)
chandumlg Feb 13, 2024
a8b8f23
chore: braze proxy v1 test (#3087)
utsabc Feb 15, 2024
fe18acd
chore: resolve conflicts
Feb 15, 2024
3a8f67b
Merge branch 'develop' into fix.network-handlers
utsabc Feb 19, 2024
b29d624
chore: resolve conflicts
Feb 19, 2024
2aa9bf2
Merge branch 'develop' into fix.network-handlers
utsabc Feb 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@
"ua-parser-js": "^1.0.37",
"unset-value": "^2.0.1",
"uuid": "^9.0.0",
"valid-url": "^1.0.9"
"valid-url": "^1.0.9",
"zod": "^3.22.4"
},
"devDependencies": {
"@commitlint/config-conventional": "^17.6.3",
Expand Down
7 changes: 4 additions & 3 deletions src/adapters/networkhandler/genericNetworkHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ const tags = require('../../v0/util/tags');
* will act as fall-fack for such scenarios.
*
*/
const responseHandler = (destinationResponse, dest) => {
const responseHandler = (responseParams) => {
const { destinationResponse, destType } = responseParams;
const { status } = destinationResponse;
const message = `[Generic Response Handler] Request for destination: ${dest} Processed Successfully`;
const message = `[Generic Response Handler] Request for destination: ${destType} Processed Successfully`;
// if the response from destination is not a success case build an explicit error
if (!isHttpStatusSuccess(status)) {
throw new NetworkError(
`[Generic Response Handler] Request failed for destination ${dest} with status: ${status}`,
`[Generic Response Handler] Request failed for destination ${destType} with status: ${status}`,
status,
{
[tags.TAG_NAMES.ERROR_TYPE]: getDynamicErrorType(status),
Expand Down
27 changes: 16 additions & 11 deletions src/controllers/delivery.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
/* eslint-disable prefer-destructuring */
/* eslint-disable sonarjs/no-duplicate-string */
import { Context } from 'koa';
import { isDefinedAndNotNullAndNotEmpty } from '@rudderstack/integrations-lib';
import { MiscService } from '../services/misc';
import {
DeliveriesResponse,
DeliveryResponse,
DeliveryV1Response,
DeliveryV0Response,
ProcessorTransformationOutput,
ProxyDeliveriesRequest,
ProxyDeliveryRequest,
ProxyV0Request,
ProxyV1Request,
} from '../types/index';
import { ServiceSelector } from '../helpers/serviceSelector';
import { DeliveryTestService } from '../services/delivertTest/deliveryTest';
Expand All @@ -22,9 +23,9 @@ const NON_DETERMINABLE = 'Non-determinable';
export class DeliveryController {
public static async deliverToDestination(ctx: Context) {
logger.debug('Native(Delivery):: Request to transformer::', JSON.stringify(ctx.request.body));
let deliveryResponse: DeliveryResponse;
let deliveryResponse: DeliveryV0Response;
const requestMetadata = MiscService.getRequestMetadata(ctx);
const deliveryRequest = ctx.request.body as ProxyDeliveryRequest;
const deliveryRequest = ctx.request.body as ProxyV0Request;
const { destination }: { destination: string } = ctx.params;
const integrationService = ServiceSelector.getNativeDestinationService();
try {
Expand All @@ -33,7 +34,7 @@ export class DeliveryController {
destination,
requestMetadata,
'v0',
)) as DeliveryResponse;
)) as DeliveryV0Response;
} catch (error: any) {
const { metadata } = deliveryRequest;
const metaTO = integrationService.getTags(
Expand All @@ -57,9 +58,9 @@ export class DeliveryController {

public static async deliverToDestinationV1(ctx: Context) {
logger.debug('Native(Delivery):: Request to transformer::', JSON.stringify(ctx.request.body));
let deliveryResponse: DeliveriesResponse;
let deliveryResponse: DeliveryV1Response;
const requestMetadata = MiscService.getRequestMetadata(ctx);
const deliveryRequest = ctx.request.body as ProxyDeliveriesRequest;
const deliveryRequest = ctx.request.body as ProxyV1Request;
const { destination }: { destination: string } = ctx.params;
const integrationService = ServiceSelector.getNativeDestinationService();
try {
Comment on lines 58 to 66
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [61-72]

In the deliverToDestinationV1 method, the handling of DeliveryV1Response and ProxyV1Request is consistent with the PR's goal. However, the access to metadata[0] assumes that the metadata array is not empty. It's recommended to add a check for the array's length to avoid potential runtime errors.

- metadata[0].destinationId || NON_DETERMINABLE,
+ metadata?.[0]?.destinationId || NON_DETERMINABLE,
- metadata[0].workspaceId || NON_DETERMINABLE,
+ metadata?.[0]?.workspaceId || NON_DETERMINABLE,

Expand All @@ -68,7 +69,7 @@ export class DeliveryController {
destination,
requestMetadata,
'v1',
)) as DeliveriesResponse;
)) as DeliveryV1Response;
} catch (error: any) {
const { metadata } = deliveryRequest;
const metaTO = integrationService.getTags(
Expand All @@ -84,7 +85,11 @@ export class DeliveryController {
);
}
ctx.body = { output: deliveryResponse };
ControllerUtility.deliveryPostProcess(ctx, deliveryResponse.status);
if (isDefinedAndNotNullAndNotEmpty(deliveryResponse.authErrorCategory)) {
ControllerUtility.deliveryPostProcess(ctx, deliveryResponse.status);
} else {
ControllerUtility.deliveryPostProcess(ctx);
}

logger.debug('Native(Delivery):: Response from transformer::', JSON.stringify(ctx.body));
return ctx;
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/DestinationService.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
DeliveryResponse,
DeliveryV0Response,
MetaTransferObject,
ProcessorTransformationRequest,
ProcessorTransformationResponse,
Expand All @@ -8,7 +8,7 @@ import {
UserDeletionRequest,
UserDeletionResponse,
ProxyRequest,
DeliveriesResponse,
DeliveryV1Response,
} from '../types/index';

export interface DestinationService {
Expand Down Expand Up @@ -49,7 +49,7 @@ export interface DestinationService {
destinationType: string,
requestMetadata: NonNullable<unknown>,
version: string,
): Promise<DeliveryResponse | DeliveriesResponse>;
): Promise<DeliveryV0Response | DeliveryV1Response>;

processUserDeletion(
requests: UserDeletionRequest[],
Expand Down
6 changes: 3 additions & 3 deletions src/services/comparator.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* eslint-disable class-methods-use-this */
import { DestinationService } from '../interfaces/DestinationService';
import {
DeliveriesResponse,
DeliveryResponse,
DeliveryV0Response,
DeliveryV1Response,
Destination,
ErrorDetailer,
MetaTransferObject,
Expand Down Expand Up @@ -370,7 +370,7 @@ export class ComparatorService implements DestinationService {
destinationType: string,
requestMetadata: NonNullable<unknown>,
version: string,
): Promise<DeliveryResponse | DeliveriesResponse> {
): Promise<DeliveryV0Response | DeliveryV1Response> {
const primaryResplist = await this.primaryService.deliver(
event,
destinationType,
Expand Down
6 changes: 3 additions & 3 deletions src/services/destination/cdkV1Integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'path';
import { TransformationError } from '@rudderstack/integrations-lib';
import { DestinationService } from '../../interfaces/DestinationService';
import {
DeliveryResponse,
DeliveryV0Response,
ErrorDetailer,
MetaTransferObject,
ProcessorTransformationRequest,
Expand All @@ -14,7 +14,7 @@ import {
UserDeletionRequest,
UserDeletionResponse,
ProxyRequest,
DeliveriesResponse,
DeliveryV1Response,
} from '../../types/index';
import { DestinationPostTransformationService } from './postTransformation';
import tags from '../../v0/util/tags';
Expand Down Expand Up @@ -121,7 +121,7 @@ export class CDKV1DestinationService implements DestinationService {
_event: ProxyRequest,
_destinationType: string,
_requestMetadata: NonNullable<unknown>,
): Promise<DeliveryResponse | DeliveriesResponse> {
): Promise<DeliveryV0Response | DeliveryV1Response> {
throw new TransformationError('CDV1 Does not Implement Delivery Routine');
}

Expand Down
6 changes: 3 additions & 3 deletions src/services/destination/cdkV2Integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { TransformationError } from '@rudderstack/integrations-lib';
import { processCdkV2Workflow } from '../../cdk/v2/handler';
import { DestinationService } from '../../interfaces/DestinationService';
import {
DeliveryResponse,
DeliveryV0Response,
ErrorDetailer,
MetaTransferObject,
ProcessorTransformationRequest,
Expand All @@ -16,7 +16,7 @@ import {
UserDeletionRequest,
UserDeletionResponse,
ProxyRequest,
DeliveriesResponse,
DeliveryV1Response,
} from '../../types/index';
import tags from '../../v0/util/tags';
import { DestinationPostTransformationService } from './postTransformation';
Expand Down Expand Up @@ -170,7 +170,7 @@ export class CDKV2DestinationService implements DestinationService {
_event: ProxyRequest,
_destinationType: string,
_requestMetadata: NonNullable<unknown>,
): Promise<DeliveryResponse | DeliveriesResponse> {
): Promise<DeliveryV0Response | DeliveryV1Response> {
throw new TransformationError('CDKV2 Does not Implement Delivery Routine');
}

Expand Down
38 changes: 18 additions & 20 deletions src/services/destination/nativeIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import groupBy from 'lodash/groupBy';
import cloneDeep from 'lodash/cloneDeep';
import { DestinationService } from '../../interfaces/DestinationService';
import {
DeliveryResponse,
DeliveryV0Response,
ErrorDetailer,
MetaTransferObject,
ProcessorTransformationRequest,
Expand All @@ -16,9 +16,9 @@ import {
UserDeletionRequest,
UserDeletionResponse,
ProxyRequest,
ProxyDeliveriesRequest,
ProxyDeliveryRequest,
DeliveriesResponse,
ProxyV0Request,
ProxyV1Request,
DeliveryV1Response,
DeliveryJobState,
} from '../../types/index';
import { DestinationPostTransformationService } from './postTransformation';
Expand Down Expand Up @@ -181,7 +181,7 @@ export class NativeIntegrationDestinationService implements DestinationService {
destinationType: string,
_requestMetadata: NonNullable<unknown>,
version: string,
): Promise<DeliveryResponse | DeliveriesResponse> {
): Promise<DeliveryV0Response | DeliveryV1Response> {
try {
const { networkHandler, handlerVersion } = networkHandlerFactory.getNetworkHandler(
destinationType,
Expand All @@ -191,24 +191,22 @@ export class NativeIntegrationDestinationService implements DestinationService {
const processedProxyResponse = networkHandler.processAxiosResponse(rawProxyResponse);
let rudderJobMetadata =
version.toLowerCase() === 'v1'
? (deliveryRequest as ProxyDeliveriesRequest).metadata
: (deliveryRequest as ProxyDeliveryRequest).metadata;
? (deliveryRequest as ProxyV1Request).metadata
: (deliveryRequest as ProxyV0Request).metadata;

if (version.toLowerCase() === 'v1' && handlerVersion.toLowerCase() === 'v0') {
rudderJobMetadata = rudderJobMetadata[0];
}

let responseProxy = networkHandler.responseHandler(
{
...processedProxyResponse,
rudderJobMetadata,
},
destinationType,
);
const responseParams = {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the core part where the change has been done which affects all network handlers

destinationResponse: processedProxyResponse,
rudderJobMetadata,
destType: destinationType,
};
let responseProxy = networkHandler.responseHandler(responseParams);
// Adaption Logic for V0 to V1
if (handlerVersion.toLowerCase() === 'v0' && version.toLowerCase() === 'v1') {
const v0Response = responseProxy as DeliveryResponse;
const jobStates = (deliveryRequest as ProxyDeliveriesRequest).metadata.map(
const v0Response = responseProxy as DeliveryV0Response;
const jobStates = (deliveryRequest as ProxyV1Request).metadata.map(
(metadata) =>
({
error: JSON.stringify(v0Response.destinationResponse?.response),
Expand All @@ -221,7 +219,7 @@ export class NativeIntegrationDestinationService implements DestinationService {
status: v0Response.status,
message: v0Response.message,
authErrorCategory: v0Response.authErrorCategory,
} as DeliveriesResponse;
} as DeliveryV1Response;
}
return responseProxy;
} catch (err: any) {
Expand All @@ -236,10 +234,10 @@ export class NativeIntegrationDestinationService implements DestinationService {
);

if (version.toLowerCase() === 'v1') {
metaTO.metadatas = (deliveryRequest as ProxyDeliveriesRequest).metadata;
metaTO.metadatas = (deliveryRequest as ProxyV1Request).metadata;
return DestinationPostTransformationService.handlevV1DeliveriesFailureEvents(err, metaTO);
}
metaTO.metadata = (deliveryRequest as ProxyDeliveryRequest).metadata;
metaTO.metadata = (deliveryRequest as ProxyV0Request).metadata;
return DestinationPostTransformationService.handleDeliveryFailureEvents(err, metaTO);
}
}
Expand Down
24 changes: 16 additions & 8 deletions src/services/destination/postTransformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import {
ProcessorTransformationResponse,
RouterTransformationResponse,
ProcessorTransformationOutput,
DeliveryResponse,
DeliveryV0Response,
MetaTransferObject,
UserDeletionResponse,
DeliveriesResponse,
DeliveryV1Response,
DeliveryJobState,
} from '../../types/index';
import { generateErrorObject } from '../../v0/util';
Expand Down Expand Up @@ -75,7 +75,13 @@ export class DestinationPostTransformationService {
): RouterTransformationResponse[] {
const resultantPayloads: RouterTransformationResponse[] = cloneDeep(transformedPayloads);
resultantPayloads.forEach((resultantPayload) => {
if (resultantPayload.batchedRequest && resultantPayload.batchedRequest.userId) {
if (Array.isArray(resultantPayload.batchedRequest)) {
resultantPayload.batchedRequest.forEach((batchedRequest) => {
if (batchedRequest.userId) {
batchedRequest.userId = `${batchedRequest.userId}`;
}
});
} else if (resultantPayload.batchedRequest && resultantPayload.batchedRequest.userId) {
resultantPayload.batchedRequest.userId = `${resultantPayload.batchedRequest.userId}`;
}
});
Expand Down Expand Up @@ -145,7 +151,7 @@ export class DestinationPostTransformationService {
public static handleDeliveryFailureEvents(
error: any,
metaTo: MetaTransferObject,
): DeliveryResponse {
): DeliveryV0Response {
const errObj = generateErrorObject(error, metaTo.errorDetails, false);
const resp = {
status: errObj.status,
Expand All @@ -155,7 +161,7 @@ export class DestinationPostTransformationService {
...(errObj.authErrorCategory && {
authErrorCategory: errObj.authErrorCategory,
}),
} as DeliveryResponse;
} as DeliveryV0Response;

ErrorReportingService.reportError(error, metaTo.errorContext, resp);
return resp;
Expand All @@ -164,7 +170,7 @@ export class DestinationPostTransformationService {
public static handlevV1DeliveriesFailureEvents(
error: FixMe,
metaTo: MetaTransferObject,
): DeliveriesResponse {
): DeliveryV1Response {
const errObj = generateErrorObject(error, metaTo.errorDetails, false);
const metadataArray = metaTo.metadatas;
if (!Array.isArray(metadataArray)) {
Expand All @@ -186,10 +192,12 @@ export class DestinationPostTransformationService {
const resp = {
response: responses,
statTags: errObj.statTags,
authErrorCategory: errObj.authErrorCategory,
message: errObj.message.toString(),
status: errObj.status,
} as DeliveriesResponse;
...(errObj.authErrorCategory && {
authErrorCategory: errObj.authErrorCategory,
}),
} as DeliveryV1Response;

ErrorReportingService.reportError(error, metaTo.errorContext, resp);
return resp;
Expand Down
Loading
Loading