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 nested array data on Monitor.put #39

Merged
merged 1 commit into from
May 2, 2024

Conversation

alexpresthus
Copy link
Contributor

The issue

In the method Monitor.put, the assertion typeof data == 'object' returns true whether data is an Array or not:
https://github.com/cronitorio/cronitor-js/blob/master/lib/monitor.js#L14

static async put(data, rollback = false) {
        if (typeof data == 'object') {
            data = [data];
        } else {
            throw new Errors.MonitorNotCreated('Invalid monitor data.');
        }

This causes data to be reassigned as a nested array when Monitor.put is called with an array as the data parameter, which causes the monitors value in the data of the PUT request to be a nesten array:

const resp = await this._api.axios.put(this._api.monitorUrl(), { monitors: data, rollback });

This behavior results in an HTTP 500 Internal Server Error from the API.

Example using config:

# cronitor.yaml
jobs:
  job-one:
    schedule: 0 2 * * *
    notify:
      - default
    platform: node-cron
    script: job:job-one

  job-two:
    schedule: 30 3,16 * * *
    notify:
      - default
    platform: node-cron
    script: job:job-two

Results in the following AxiosResponse from the API:

AxiosError: Request failed with status code 500
       at settle (/home/alex/krogsveen/git/workers/node_modules/.pnpm/[email protected]/node_modules/axios/dist/node/axios.cjs:1966:12)
       at IncomingMessage.handleStreamEnd (/home/alex/krogsveen/git/workers/node_modules/.pnpm/[email protected]/node_modules/axios/dist/node/axios.cjs:3065:11)
       at IncomingMessage.emit (node:events:530:35)
       at IncomingMessage.emit (node:domain:488:12)
       at endReadableNT (node:internal/streams/readable:1696:12)
       at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
       at Axios.request (/home/alex/krogsveen/git/workers/node_modules/.pnpm/[email protected]/node_modules/axios/dist/node/axios.cjs:3876:41)
       at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
       at async Monitor.put (/home/alex/krogsveen/git/workers/node_modules/.pnpm/[email protected]/node_modules/cronitor/lib/monitor.js:22:26)
       at async Cronitor.applyConfig (/home/alex/krogsveen/git/workers/node_modules/.pnpm/[email protected]/node_modules/cronitor/lib/cronitor.js:56:20)
       at async initializeJobSchedule (/home/alex/krogsveen/git/workers/dist/jobs/loaders/job-schedule.loader.js:37:5)
       at async /home/alex/krogsveen/git/workers/dist/jobs/app.js:18:9 {
    code: 'ERR_BAD_RESPONSE',
    config: {
        ...,
        method: 'put',
        url: 'https://cronitor.io/api/monitors',
        data: '{"monitors":[[{"schedule":"0 2 * * *","notify":["default"],"platform":"node-cron","script":"job:job-one","key":"job-one","type":"job"},{"schedule":"30 3,16 * * *","notify":["default"],"platform":"node-cron","script":"job:job-two","key":"job-two","type":"job"}]],"rollback":false}'
    },
    res: IncomingMessage {
        ...,
        statusCode: 500,
        statusMessage: 'Internal Server Error',
        ...,
      },
    ...,
}

Which outputs the error message from applyConfig:

Error applying config: Error: Request failed with status code 500

The fix

Add a condition if (!Array.isArray(data)) to make sure the data param is only reassigned into an array if the initial data param is not an array.

Make sure the `data` param is only reassigned into an array if the
initial `data` param is not an array.

> In the method `Monitor.put`, the assertion `typeof data == 'object'` returns
> `true` whether `data` is an Array or not. This causes `data` to be reassigned
> as a nested array when `Monitor.put` is called with an array as the
> `data` parameter, resulting in an HTTP 500 Error from the API.
@aflanagan aflanagan merged commit 893da37 into cronitorio:master May 2, 2024
4 checks passed
@aflanagan
Copy link
Contributor

Thanks for the bug fix @alexpresthus!

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.

2 participants