Skip to content

Commit

Permalink
Fix retry logic (#14)
Browse files Browse the repository at this point in the history
* Fix retry scheme; add termination condition

* Update error reporting to capture error on form request

* Update node version in cd-test pipeline

* Update yarn lock file

* Update tests
  • Loading branch information
peterMuriuki authored Nov 5, 2024
1 parent 17f3224 commit e95461e
Show file tree
Hide file tree
Showing 11 changed files with 178 additions and 122 deletions.
40 changes: 28 additions & 12 deletions .github/workflows/ci-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,42 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [16.x]
node-version: [20.x]
steps:
- uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
- name: Checkout repository
uses: actions/checkout@v4

# Set up Node.js environment
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'
cache-dependency-path: '**/yarn.lock'
- name: Enable corepack
node-version: '20.x' # Ensures Node.js version is 20 or higher]

# Enable Corepack for Yarn v3 support
- name: Enable Corepack
run: corepack enable
- name: Insall npm packages

# Re- set up Node.js environment with Corepack enabled and cache
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '20.x' # Ensures Node.js version is 20 or higher
cache: 'yarn' # Caches Yarn dependencies

# Install dependencies using Yarn
- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Install system wide playwright dependencies
run: npx playwright install --with-deps

- name: Lint
run: yarn lint
- name: Run all tests
run: yarn test
- uses: actions/upload-artifact@v3

- name: Run core tests
run: yarn turbo run test --filter @onaio/symbology-calc-core

- uses: actions/upload-artifact@v4
if: always()
with:
name: playwright-report
Expand Down
47 changes: 20 additions & 27 deletions apps/web/src/routes/workflows/reports/[slug]/+page.svelte
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
<script lang="ts">
import type { PageData } from './$types';
import { entries, range } from 'lodash-es';
import {
formatTimestamp,
formatTriggerDuration,
parseForTable
} from './utils';
import { range } from 'lodash-es';
import { formatTriggerDuration, parseForTable } from './utils';
import PageHeader from '$lib/shared/components/PageHeader.svelte';
import { goto, invalidate } from '$app/navigation';
import { goto } from '$app/navigation';
import { toast } from '@zerodevx/svelte-toast';
import { convertCronToHuman } from '../../utils';
export let data: PageData;
// callback for manual trigger user action.
Expand Down Expand Up @@ -68,15 +64,15 @@
const getNotModifiedReasons = (metric: any) => {
const onlyNotModified = { ...metric.facilitiesEvaluated.notModified };
delete onlyNotModified.total
delete onlyNotModified.total;
return onlyNotModified;
}
};
const getNotEvaluatedReasons = (metric: any) => {
const notEvaluatedReasons = { ...metric.facilitiesNotEvaluated };
delete notEvaluatedReasons.total
delete notEvaluatedReasons.total;
return notEvaluatedReasons;
}
};
</script>

<!-- <svelte:head>
Expand Down Expand Up @@ -198,6 +194,14 @@
<dt class="col-sm-9">No. of facilities NOT evaluated</dt>
<dd class="col-sm-3">{metric?.facilitiesNotEvaluated.total}</dd>
</dl>
{@const generalErrors = metric?.generalErrors ?? []}
<div>
{#each generalErrors as error}
<div class="alert alert-danger" role="alert">
{error}
</div>
{/each}
</div>
<!-- parse for facilities evaluated breakdown -->
<div class="card mb-3">
<div class="card-body">
Expand Down Expand Up @@ -230,7 +234,6 @@
</td>
</tr>
</tbody>

</table>

<p class="fw-bold">Not Modified.({metric.facilitiesEvaluated.notModified.total})</p>
Expand All @@ -246,9 +249,7 @@
{#each Object.entries(getNotModifiedReasons(metric)) as row}
<tr>
<td>{row[0]}</td>
<td
> {row[1].description}</td
>
<td> {row[1].description}</td>
<td>{row[1].total}</td>
</tr>
{/each}
Expand All @@ -259,10 +260,7 @@
</td>
</tr>
</tbody>

</table>


</div>
</div>

Expand All @@ -284,24 +282,19 @@
{#each Object.entries(getNotEvaluatedReasons(metric)) as row}
<tr>
<td>{row[0]}</td>
<td
> {row[1].description}</td
>
<td> {row[1].description}</td>
<td>{row[1].total}</td>
</tr>
{/each}

<tr>
<td colspan="2">Total</td>
<td>
{metric.facilitiesNotEvaluated.total}
</td>
</tr>
</tbody>

</table>


</div>
</div>
{/if}
Expand All @@ -317,7 +310,7 @@
font-weight: 500;
}
th{
th {
font-weight: 500;
}
</style>
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "sample",
"name": "custom-scripting",
"version": "0.0.0",
"private": true,
"workspaces": [
Expand Down Expand Up @@ -46,7 +46,7 @@
]
},
"engines": {
"node": ">=16.0.0"
"node": ">=18.0.0"
},
"packageManager": "yarn@3.2.3"
"packageManager": "yarn@3.6.1"
}
1 change: 1 addition & 0 deletions packages/core/src/evaluator/configRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export class ConfigRunner {
abortableBlock: {
const regForm = await service.fetchSingleForm(regFormId);
if (regForm.isFailure) {
reporter.updateGeneralError(`Fetching form with id ${regFormId} failed due to : ${regForm.error}`)
yield reporter.generateJsonReport(true);
return;
}
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/evaluator/metricReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export class ReportMetric {
private facilitiesEvaluatedModified: Record<string, Record<string, number>> = {};
private facilitiesEvaluatedNotModified: Record<string, number> = {};
private configId: string;
private generalErrors?: Array<string>

constructor(configId: string) {
this.configId = configId;
Expand Down Expand Up @@ -42,6 +43,13 @@ export class ReportMetric {
}
}

public updateGeneralError(errorMessage: string){
if(this.generalErrors === undefined){
this.generalErrors = []
}
this.generalErrors.push(errorMessage)
}

public updateTotalFacilities(value: number) {
this.totalFacilities = value;
}
Expand Down Expand Up @@ -127,7 +135,8 @@ export class ReportMetric {
},
facilitiesNotEvaluated: {
...notEvaluated
}
},
generalErrors: this.generalErrors,
};
}
}
Expand Down
20 changes: 11 additions & 9 deletions packages/core/src/evaluator/tests/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { OnaApiService } from '../../services/onaApi/services';
import * as services from '../../services/onaApi/services';
import { createConfigs, form3623Submissions } from './fixtures/fixtures';
import { transformFacility, createMetricFactory } from '../helpers/utils';
import { colorDeciderFactory } from '../../helpers/utils';
Expand All @@ -9,6 +9,8 @@ import {
} from '../../constants';
import { RegFormSubmission } from '../../helpers/types';

const {OnaApiService} = services

// eslint-disable-next-line @typescript-eslint/no-var-requires
const nock = require('nock');

Expand Down Expand Up @@ -132,7 +134,7 @@ describe('transform facility tests', () => {
query: `{"facility": ${regFomSubmission._id}}`, // filter visit submissions for this facility
sort: `{"${dateOfVisitAccessor}": -1}`
})
.reply(502, { message: 'error' });
.reply(400, { message: 'error' }).persist();

// after attempt
nock(baseUrl)
Expand All @@ -143,14 +145,14 @@ describe('transform facility tests', () => {
query: `{"facility": ${regFomSubmission._id}}`, // filter visit submissions for this facility
sort: `{"${dateOfVisitAccessor}": -1}`
})
.reply(400, { message: 'error' });
.reply(400, { message: 'error' }).persist();

nock(baseUrl)
.post(`/${editSubmissionEndpoint}`, {
id: '3623',
submission: {
...regFomSubmission,
'marker-color': 'green',
'marker-color': 'red',
meta: {
instanceID: 'uuid:0af4f147-d5fd-486a-bf76-d1bf850cc976',
deprecatedID: regFomSubmission['meta/instanceID']
Expand All @@ -172,18 +174,18 @@ describe('transform facility tests', () => {
logger
);

expect(response).toEqual({
expect(response).toMatchObject({
_value: undefined,
detail: {
code: 'ECODE3',
recsAffected: 0
},
error: '400: {"message":"error"}: Network request failed.',
error: expect.any(String),
isFailure: true,
isSuccess: false
});
expect(response.error).toEqual('400: {"message":"error"}: Network request failed.');
});
expect(response.error).toContain("Request failed for | URL: http://sample.com/api/v1/data/3624?page_size=1&page=1&query=%7B%22facility%22%3A+304870%7D&sort=%7B%22endtime%22%3A+-1%7D | Status: 400");
}, 20000);

it('cancelling request works', async () => {
const apiToken = 'apiToken';
Expand Down Expand Up @@ -241,7 +243,7 @@ describe('transform facility tests', () => {
code: 'ECODE3',
recsAffected: 0
},
error: 'AbortError: The user aborted a request..',
"error": "Error Name: AbortError | Message: The user aborted a request.",
isFailure: true,
isSuccess: false
});
Expand Down
9 changes: 4 additions & 5 deletions packages/core/src/evaluator/tests/pipelinesController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,10 @@ it('error when fetching the registration form', async () => {
expect(loggerMock.mock.calls).toEqual([
[
{
level: 'error',
message:
'Operation to fetch form: 3623, failed with err: Error: 400: Could not find form with id: Network request failed.'
}
]
"level": "error",
"message": "Operation to fetch form: 3623, failed with err: Request failed for | URL: https://test-api.ona.io/api/v1/forms/3623 | Status: 400",
},
],
]);

expect(nock.pendingMocks()).toEqual([]);
Expand Down
26 changes: 24 additions & 2 deletions packages/core/src/helpers/Result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,18 @@ export class Result<T> {
return new Result<U>(true, undefined, value, resultDetail);
}

public static fail<U>(error: string, detail?: ResultDetail | ResultCodes): Result<U> {
public static fail<U>(error: Error | string, detail?: ResultDetail | ResultCodes): Result<U> {
let errorDesc = error as string
if(typeof error == 'object'){
errorDesc = getErrorDescription(error)
}
let resultDetail;
if (typeof detail === 'string') {
resultDetail = { code: detail };
} else {
resultDetail = detail;
}
return new Result<U>(false, error, undefined, resultDetail);
return new Result<U>(false, errorDesc, undefined, resultDetail);
}

public static bubbleFailure<NewType, OldType>(result: Result<OldType>): Result<NewType> {
Expand All @@ -155,3 +159,21 @@ export class Result<T> {
return this.fail<NewType>(result.error, result.detail);
}
}


function getErrorDescription(error: Error){
const { name, message, cause } = error;
const code = (cause as Record<string, unknown> | undefined)?.code as string | undefined
const causeMessage = (cause as Record<string, unknown> | undefined)?.message as string | undefined
const errorDetails = [];

if (name) errorDetails.push(`Error Name: ${name}`);
if (message) errorDetails.push(`Message: ${message}`);
if (code) errorDetails.push(`Code: ${code}`);
if (causeMessage) errorDetails.push(`Cause Message: ${causeMessage}`);


return errorDetails.length > 0
? errorDetails.join(' | ')
: "An unknown network error occurred.";
}
1 change: 1 addition & 0 deletions packages/core/src/helpers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export interface Metric {
notModifiedWithError: number;
modified: number;
totalSubmissions?: number;
generalErrors?: string[]
}

export interface WriteMetric {
Expand Down
Loading

0 comments on commit e95461e

Please sign in to comment.