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

[Feat] Removing Axios for Native Fetch #1278

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

sapiderman
Copy link
Contributor

@sapiderman sapiderman commented Apr 19, 2024

Monika Pull Request (PR)

What feature/issue does this PR add

  1. Implements Remove Axios package and defaults to Node.js fetch #1248
  2. Replacing with native fetch main Monika http transactions

How did you implement / how did you fix it

  1. Remove sendHttp wrappers with native fetch

How to test

  1. npm start -- -c test.yaml should work
  2. npm test should all pass
  3. monika -c https://raw.githubusercontent.com/hyperjumptech/monika/main/config_sample/config.desktop.example.yml should be fetched properly
  4. monika --auto-update patch should work

url: downloadUri,
responseType: 'stream',
})

const { data: checksum }: { data: string } = await sendHttpRequest({
const { data: downloadStream } = (await resp.json()) as { data: Stream }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can get stream from resp.body instead of resp.json() ?

ref https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Using_readable_streams

}

private async report(): Promise<void> {
// Create a task data object
const taskData = {
apiKey: this.apiKey,
httpClient: this.httpClient,
// httpClient: this.httpClient,
Copy link
Contributor

Choose a reason for hiding this comment

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

dead code ?

} event for Alert ID: ${alertId} failed. ${(error as Error).message}`
)
try {
const resp = await sendHttpRequestFetch({
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this API call is missing x-api-key header ?

maybe we can create a reusable function inside symon client, something like

private fetcher() {
  sendHttpRequestFetch({
    // base url
    // common headers
  })
}

const resp = await sendHttpRequestFetch({
url: `${this.url}/api/${this.apiVersion}/${this.monikaId}/probes`,
method: 'GET',
headers: {
Copy link
Contributor

Choose a reason for hiding this comment

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

another one with no x-api-key header

url: `${this.url}/api/${this.apiVersion}/monika/client-handshake`,
method: 'POST',
body: JSON.stringify(handshakeData),
headers: {
Copy link
Contributor

Choose a reason for hiding this comment

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

another one without x-api-key header

@sapiderman sapiderman marked this pull request as draft May 15, 2024 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants