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(echo): Replace faulty instanceof check with static code check #5678

Merged
merged 9 commits into from
Jun 4, 2024
4 changes: 2 additions & 2 deletions .github/actions/start-localstack/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ runs:
AWS_EC2_METADATA_DISABLED: true
working-directory: docker/local
run: |
docker-compose -f docker-compose.localstack.yml up -d
docker-compose -f docker-compose.e2e.yml up -d
sleep 10
max_retry=30
counter=0
Expand All @@ -28,5 +28,5 @@ runs:
echo "Trying again. Try #$counter"
((counter++))
done
docker-compose -f docker-compose.localstack.yml logs --tail="all"
docker-compose -f docker-compose.e2e.yml logs --tail="all"
aws --endpoint-url=http://127.0.0.1:4566 --cli-connect-timeout 600 s3 mb s3://novu-test
18 changes: 18 additions & 0 deletions docker/local/docker-compose.e2e.yml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This addition was needed to fix the failing CI pipeline, it was accidentally removed in a previous PR.

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
version: '3.1'

services:
localstack:
container_name: "${LOCALSTACK_DOCKER_NAME-localstack_main}"
image: "localstack/localstack:0.14.5"
network_mode: bridge
environment:
- SERVICES=s3
ports:
- "${DOCKER_LOCALSTACK_PORT:-4566}:4566"
volumes:
- "${TMPDIR:-/tmp/localstack}:/tmp/localstack"
- "/var/run/docker.sock:/var/run/docker.sock"
healthcheck:
test: "bash -c 'AWS_ACCESS_KEY_ID=test AWS_SECRET_ACCESS_KEY=test aws --endpoint-url=http://127.0.0.1:4566 s3 ls'"
retries: 5
interval: 10s
2 changes: 1 addition & 1 deletion packages/create-novu-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"scripts": {
"start": "node dist/index.js",
"dev": "ncc build ./index.ts -w -o dist/",
"prerelease": "node ../../scripts/rm.mjs dist",
"prerelease": "rimraf dist",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Small fix to ensure installation passes without error.

"release": "ncc build ./index.ts -o ./dist/ --minify --no-cache --no-source-map-register",
"build": "pnpm release",
"lint-fix": "pnpm prettier -w --plugin prettier-plugin-tailwindcss 'templates/*-tw/{ts,js}/{app,pages}/**/*.{js,ts,tsx}'",
Expand Down
7 changes: 5 additions & 2 deletions packages/echo/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@novu/echo",
"version": "0.24.3-alpha.0",
"version": "0.24.3-alpha.2",
"description": "The Code-First Notifications Workflow SDK.",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
Expand All @@ -9,7 +9,10 @@
"access": "public"
},
"private": false,
"repository": "https://github.com/novuhq/novu",
"repository": {
"type": "git",
"url": "git+https://github.com/novuhq/novu.git"
},
"scripts": {
"preinstall": "npx only-allow pnpm",
"test": "jest",
Expand Down
9 changes: 7 additions & 2 deletions packages/echo/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createHmac } from 'node:crypto';

import { Echo } from './client';
import {
ErrorCodeEnum,
GetActionEnum,
HttpHeaderKeysEnum,
HttpMethodEnum,
Expand Down Expand Up @@ -261,8 +262,12 @@ export class EchoRequestHandler<Input extends any[] = any[], Output = any> {
}
}

private handleError(error: any): IActionResponse {
if (error instanceof EchoError) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥 the instanceof in cross bundles

private isClientError(error: unknown): error is EchoError {
return Object.values(ErrorCodeEnum).includes((error as EchoError).code);
}

private handleError(error: unknown): IActionResponse {
if (this.isClientError(error)) {
if (error.statusCode === HttpStatusEnum.INTERNAL_SERVER_ERROR) {
// eslint-disable-next-line no-console
console.error(error);
Expand Down
Loading