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

Enhanced retry mechanism & test case added #295

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
78 changes: 42 additions & 36 deletions __tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,45 +11,51 @@
const { ServerSideUtils } = require('facebook-nodejs-business-sdk');
const sha256 = require('js-sha256');


describe('ServerSideUtils', () => {
describe('normalizeAndHash', () => {
test('normalizes and hashes the input', () => {
const value = ' Eg ';
const expected = sha256('eg');
const actual = ServerSideUtils.normalizeAndHash(value, 'ln');

expect(actual).toBe(expected);
});

test('does not double hash sha256', () => {
const value = sha256('11234567890');
const actual = ServerSideUtils.normalizeAndHash(value, 'ph');

expect(actual).toBe(value);
});

test('does not double hash md5', () => {
const value = '2a6a84e9e44441afbd75cc19ce28be37';
const actual = ServerSideUtils.normalizeAndHash(value, 'ln');

expect(actual).toBe(value);
});

test('validates the input', () => {
const normalizeAndHash = () => {
ServerSideUtils.normalizeAndHash('1234', 'em');
}
expect(normalizeAndHash).toThrow();
});
describe('normalizeAndHash', () => {
test('normalizes and hashes the input', () => {
const value = ' Eg ';
const expected = sha256('eg');
const actual = ServerSideUtils.normalizeAndHash(value, 'ln');

expect(actual).toBe(expected);
});

test('does not double hash sha256', () => {
const value = sha256('11234567890');
const actual = ServerSideUtils.normalizeAndHash(value, 'ph');

expect(actual).toBe(value);
});

test('does not double hash md5', () => {
const value = '2a6a84e9e44441afbd75cc19ce28be37';
const actual = ServerSideUtils.normalizeAndHash(value, 'ln');

expect(actual).toBe(value);
});

test('validates the input', () => {
const normalizeAndHash = () => {
ServerSideUtils.normalizeAndHash('1234', 'em');
};
expect(normalizeAndHash).toThrow();
});
test('handles empty string correctly', () => {
const value = '';
const expected = sha256(value);
const actual = ServerSideUtils.normalizeAndHash(value, 'ln');

expect(actual).toBe(expected);
});
});

describe('toSHA256', () => {
test('hashes the value', () => {
const value = 'test123';
const expected = sha256(value);
describe('toSHA256', () => {
test('hashes the value', () => {
const value = 'test123';
const expected = sha256(value);

expect(ServerSideUtils.toSHA256(value)).toBe(expected);
});
expect(ServerSideUtils.toSHA256(value)).toBe(expected);
});
});
});
2 changes: 2 additions & 0 deletions src/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ export default class FacebookAdsApi {
${Object.keys(data).length > 0 ? JSON.stringify(data) : ''}`,
);
}
console.error(`Error in ${method} request to ${url} with data: ${JSON.stringify(data)}`);
console.error('Response:', response);
Copy link
Contributor

Choose a reason for hiding this comment

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

could you remove these two lines of changes? the api.js file is heavily used and could have bigger impact.

throw new FacebookRequestError(response, method, url, data);
});
}
Expand Down
44 changes: 25 additions & 19 deletions src/video-uploader.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ class VideoUploadTransferRequestManager extends VideoUploadRequestManager {
async sendRequest(context: VideoUploadRequestContext): Object {
// Init a VideoUploadRequest
const request = new VideoUploadRequest(this._api);
var start_offset = context.startOffset;
var end_offset = context.endOffset;
let start_offset = context.startOffset;
let end_offset = context.endOffset;
const filePath = context.filePath;
const fileSize = fs.statSync(filePath).size;

Expand All @@ -223,42 +223,48 @@ class VideoUploadTransferRequestManager extends VideoUploadRequestManager {
let response = null;
// While there are still more chunks to send
const videoFileDescriptor = fs.openSync(filePath, 'r');
let retryDelay = 1000;

while (start_offset !== end_offset) {
context.startOffset = start_offset;
context.endOffset = end_offset;
let params = {
upload_phase: 'transfer',
start_offset: context.startOffset,
upload_session_id: context.sessionId,
video_file_chunk: context.videoFileChunk,
};
request.setParams(params, {
video_file_chunk: fs.createReadStream(context.filePath, {
start: context.startOffset,
end: context.endOffset - 1,
}),
});
// Send the request
request.setParams(
{
upload_phase: 'transfer',
start_offset: context.startOffset,
upload_session_id: context.sessionId,
},
{
video_file_chunk: fs.createReadStream(context.filePath, {
start: context.startOffset,
end: context.endOffset - 1,
}),
}
);

try {
response = await request.send([context.accountId, 'advideos']);
start_offset = parseInt(response['start_offset']);
end_offset = parseInt(response['end_offset']);
} catch (error) {
if (numRetry > 0) {
numRetry = Math.max(numRetry - 1, 0);
await sleep(retryDelay);
retryDelay *= 2; // Exponential backoff
continue;
}
fs.close(videoFileDescriptor, err => {});
fs.close(videoFileDescriptor, (err) => {});
throw error;
}
}

this._startOffset = start_offset;
this._endOffset = end_offset;
fs.close(videoFileDescriptor, err => {});

fs.close(videoFileDescriptor, (err) => {});
Copy link
Contributor

Choose a reason for hiding this comment

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

could you remove the empty spaces?

return response;
}

}

class VideoUploadFinishRequestManager extends VideoUploadRequestManager {
Expand Down