Skip to content

Commit

Permalink
replaced fetch with axios due to timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
f-w committed Dec 14, 2023
1 parent e46687f commit 91305a9
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 53 deletions.
Binary file modified .github/values.ocp4.dev.yaml.gpg
Binary file not shown.
1 change: 1 addition & 0 deletions .github/workflows/buildTestPublishContainerDeploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ on:
push:
branches:
- main
- pullAll
release:
types:
- published
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@nestjs/platform-express": "^10.0.0",
"@nestjs/swagger": "^7.1.8",
"async": "^3.2.4",
"axios": "^1.6.2",
"bcryptjs": "^2.4.3",
"bottleneck": "^2.19.5",
"class-transformer": "^0.5.1",
Expand Down
20 changes: 11 additions & 9 deletions src/api/notifications/notifications.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
HttpException,
HttpStatus,
Inject,
Logger,
Param,
ParseIntPipe,
Patch,
Expand All @@ -40,6 +41,7 @@ import {
ApiTags,
} from '@nestjs/swagger';
import { queue } from 'async';
import axios from 'axios';
import { Request } from 'express';
import jmespath from 'jmespath';
import { pullAll } from 'lodash';
Expand Down Expand Up @@ -300,6 +302,10 @@ export class NotificationsController extends BaseController {
this.appConfig.notification?.guaranteedBroadcastPushDispatchProcessing
) {
this.req.on('close', () => {
Logger.debug(
'request close. chunkRequestAborted',
NotificationsController.name,
);
this.chunkRequestAborted = true;
});
}
Expand Down Expand Up @@ -744,22 +750,18 @@ export class NotificationsController extends BaseController {
data.id +
'/broadcastToChunkSubscribers?start=' +
task.startIdx;
const response = await fetch(uri);
if (response.status < 300) {
try {
return await response.json();
} catch (ex) {
return response.body;
}
}
throw new HttpException(undefined, response.status);
const response = await axios.get(uri);
return response.data;
}, broadcastSubRequestBatchSize);
// re-submit task on error if
// guaranteedBroadcastPushDispatchProcessing.
// See issue #39
let failedChunks: any[] = [];
q.error((_err: any, task: any) => {
if (this.guaranteedBroadcastPushDispatchProcessing) {
Logger.debug('re-push task', NotificationsController.name);
Logger.debug(task, NotificationsController.name);
Logger.debug(_err, NotificationsController.name);
q.push(task);
} else {
data.state = 'error';
Expand Down
17 changes: 0 additions & 17 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,6 @@ const config: Record<string, any> = {
host: '0.0.0.0',
// port listen on
port: 3000,
remoting: {
rest: {
normalizeHttpPath: false,
xml: false,
handleErrors: false,
},
json: {
strict: false,
limit: '100kb',
},
urlencoded: {
extended: true,
limit: '100kb',
},
cors: false,
},
legacyExplorer: false,
adminIps: ['127.0.0.1'],
siteMinderReverseProxyIps: ['127.0.0.1'],
email: {
Expand Down
37 changes: 11 additions & 26 deletions test/notification.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import { HttpException, HttpStatus } from '@nestjs/common';
import { NestExpressApplication } from '@nestjs/platform-express';
import axios from 'axios';
import dns from 'dns';
import { merge } from 'lodash';
import nodemailer from 'nodemailer';
Expand Down Expand Up @@ -838,7 +839,7 @@ describe('POST /notifications', () => {
broadcastSubRequestBatchSize: 10,
});
appConfig.notification = newNotificationConfig;
const spiedFetch = jest.spyOn(global, 'fetch');
const spiedAxios = jest.spyOn(axios, 'get');
const res = await client
.post('/api/notifications')
.send({
Expand All @@ -856,7 +857,7 @@ describe('POST /notifications', () => {
expect(
BaseController.prototype.sendEmail as unknown as jest.SpyInstance,
).toBeCalledTimes(2);
expect(spiedFetch).toBeCalledTimes(2);
expect(spiedAxios).toBeCalledTimes(2);
const data = await notificationsService.findAll(
{
where: {
Expand All @@ -882,28 +883,12 @@ describe('POST /notifications', () => {

const spiedFetch = jest
.spyOn(global, 'fetch')
.mockImplementation(async function (...args: any[]) {
if (
args.length === 1 ||
(args.length > 1 && args[1].method === undefined)
) {
const getReq = client.get(
args[0].substring(args[0].indexOf('/api')),
);
if (args[1]) {
for (const [p, v] of Object.entries(args[1].headers as object)) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
getReq.set(p, v);
}
}
const data: any = await getReq;
return new Response(JSON.stringify(data.body));
}
if (args.length > 1 && args[1].method === 'POST') {
return new Response();
}
throw new Error();
});
.mockResolvedValue(new Response());
jest.spyOn(axios, 'get').mockImplementation(async (...args: any[]) => {
const getReq = client.get(args[0].substring(args[0].indexOf('/api')));
const data: any = await getReq;
return new Response(JSON.stringify(data.body));
});

(
BaseController.prototype.sendEmail as unknown as jest.SpyInstance
Expand Down Expand Up @@ -1032,7 +1017,7 @@ describe('POST /notifications', () => {
appConfig.notification = newNotificationConfig;

const reqStub = jest
.spyOn(global, 'fetch')
.spyOn(axios, 'get')
.mockRejectedValueOnce({ error: 'connection error' });
const res = await client
.post('/api/notifications')
Expand Down Expand Up @@ -1473,7 +1458,7 @@ describe('POST /notifications', () => {
guaranteedBroadcastPushDispatchProcessing: false,
});
appConfig.notification = newNotificationConfig;
jest.spyOn(global, 'fetch').mockRejectedValue({});
jest.spyOn(axios, 'get').mockRejectedValue({});
const res = await client
.post('/api/notifications')
.send({
Expand Down
16 changes: 15 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2355,6 +2355,15 @@ axios@^0.26.1:
dependencies:
follow-redirects "^1.14.8"

axios@^1.6.2:
version "1.6.2"
resolved "https://registry.yarnpkg.com/axios/-/axios-1.6.2.tgz#de67d42c755b571d3e698df1b6504cde9b0ee9f2"
integrity sha512-7i24Ri4pmDRfJTR7LDBhsOTtcm+9kjX5WiY1X3wIisx6G9So3pfMkEiU7emUBe46oceVImccTEM3k6C5dbVW8A==
dependencies:
follow-redirects "^1.15.0"
form-data "^4.0.0"
proxy-from-env "^1.1.0"

babel-jest@^29.6.2:
version "29.6.2"
resolved "https://registry.yarnpkg.com/babel-jest/-/babel-jest-29.6.2.tgz#cada0a59e07f5acaeb11cbae7e3ba92aec9c1126"
Expand Down Expand Up @@ -3610,7 +3619,7 @@ flatted@^3.1.0:
resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.2.7.tgz#609f39207cb614b89d0765b477cb2d437fbf9787"
integrity sha512-5nqDSxl8nn5BSNxyR3n4I6eDmbolI6WT+QqR547RwxQapgjQBmtktdP+HTBb/a/zLsbzERTONyUB5pefh5TtjQ==

follow-redirects@^1.14.8:
follow-redirects@^1.14.8, follow-redirects@^1.15.0:
version "1.15.3"
resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.3.tgz#fe2f3ef2690afce7e82ed0b44db08165b207123a"
integrity sha512-1VzOtuEM8pC9SFU1E+8KfTjZyMztRsgEfwQl44z8A25uy13jSzTj6dyK2Df52iV0vgHCfBwLhDWevLn95w5v6Q==
Expand Down Expand Up @@ -5704,6 +5713,11 @@ proxy-addr@~2.0.7:
forwarded "0.2.0"
ipaddr.js "1.9.1"

proxy-from-env@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2"
integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==

pump@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/pump/-/pump-3.0.0.tgz#b4a2116815bde2f4e1ea602354e8c75565107a64"
Expand Down

0 comments on commit 91305a9

Please sign in to comment.