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.

Sign up for GitHub

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

Jump to bottom

Chore: Remove Axios Package #1350

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Chore: Remove Axios Package #1350

wants to merge 9 commits into from

Conversation

haricnugraha
Copy link
Contributor

Monika Pull Request (PR)

What feature/issue does this PR add

Remove Axios package

How did you implement / how did you fix it

Change the implementation to built-in Node.js fetch.

How to test

  • Run npm test
  • Run Monika manually

@codecov Codecov
Copy link

codecov bot commented Feb 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 46.78899% with 58 lines in your changes missing coverage. Please review.

Project coverage is 64.51%. Comparing base (6a29470) to head (0f24669).
Report is 40 commits behind head on main.

Files with missing lines Patch % Lines
src/plugins/updater/index.ts 25.00% 30 Missing ⚠️
src/symon/index.ts 50.00% 10 Missing and 7 partials ⚠️
...ugins/visualization/atlassian-status-page/index.ts 11.11% 8 Missing ⚠️
src/components/config/parser/insomnia.ts 0.00% 0 Missing and 1 partial ⚠️
src/components/probe/prober/http/request.ts 87.50% 1 Missing ⚠️
src/plugins/sentry/index.ts 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
+ Coverage   62.51%   64.51%   +2.00%     
==========================================
  Files         112      111       -1     
  Lines        3391     3647     +256     
  Branches      591      639      +48     
==========================================
+ Hits         2120     2353     +233     
+ Misses       1079     1058      -21     
- Partials      192      236      +44

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haricnugraha haricnugraha marked this pull request as ready for review February 28, 2025 08:47
This was linked to issues Feb 28, 2025
Remove Axios package and defaults to Node.js fetch #1248
Open
Remove toggle --native-fetch #1249
Open
sapiderman
sapiderman approved these changes Mar 4, 2025
View reviewed changes
Copy link
Contributor

@sapiderman sapiderman left a comment

Choose a reason for hiding this comment

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

works for me, best implementation so far. nice work!

dennypradipta
dennypradipta requested changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@dennypradipta dennypradipta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, but needs some improvement when probing websites with invalid SSLs:

probes:
  - id: '1'
    name: GitHub
    description: Multiple
    requests:
      - url: https://expired.badssl.com/
    interval: 3
notifications:
  - id: '1'
    type: 'discord'
    data:
      url: <discord_url>

Result:
image
image

In Symon Mode, it goes to Error 99:
image

However, using --ignoreInvalidTLS works like a charm, no issues here.
image

It just throws Unknown Error without solid reason WHY it happens. Maybe you need to handle errors for invalid SSL from undici?

Probably try these domains:

@haricnugraha
Copy link
Contributor Author 

https://expired.badssl.com/

Thanks for the feedback! The issue has been resolved.

Monika Mode

Screenshot 2025-03-07 at 8 59 39 PM

Symon Mode

Screenshot 2025-03-10 at 2 49 29 PM

@sapiderman
Copy link
Contributor

latest commit good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennypradipta dennypradipta dennypradipta requested changes

@sapiderman sapiderman sapiderman approved these changes

@raosan raosan Awaiting requested review from raosan

@syamsudotdev syamsudotdev Awaiting requested review from syamsudotdev

@lukman7788 lukman7788 Awaiting requested review from lukman7788

@herenacreative herenacreative Awaiting requested review from herenacreative

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove toggle --native-fetch Remove Axios package and defaults to Node.js fetch
3 participants
@haricnugraha @sapiderman @dennypradipta