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

Use Axios's toJSON method to make shorter errors #632

Merged
merged 13 commits into from
Mar 10, 2023

Conversation

BryceStevenWilley
Copy link
Collaborator

@BryceStevenWilley BryceStevenWilley commented Dec 12, 2022

On an example erroring docassemble server (for example, this server returning a 502 to a delete project), the entire axios error will print, which is that case in 1.8k lines, most of it being garbage.

Axios has a toJSON() method on errors that should reduce length of the errors being shown, as it will not print the most annoying parts of the long errors, like the internal axios socket objects

The errors look like this now, which IMO is an improvement:

$ alkiln-takedown
ALKiln takedown: Trying to remove the docassemble Project from the testing account.
ALKiln : Trying to delete Project fake_project

ALKiln  ERROR: Ran into an error when deleting Project "fake_project"

ALKiln takedown ERROR: could not delete_project
{
    "message": "Request failed with status code 400",
    "name": "Error",
    "stack": "Error: Request failed with status code 400\n    at createError (/home/brycew/Developer/LITLab/code/ALKiln/node_modules/axios/lib/core/createError.js:16:15)\n    at settle (/home/brycew/Developer/LITLab/code/ALKiln/node_modules/axios/lib/core/settle.js:17:12)\n    at IncomingMessage.handleStreamEnd (/home/brycew/Developer/LITLab/code/ALKiln/node_modules/axios/lib/adapters/http.js:293:11)\n    at IncomingMessage.emit (node:events:377:35)\n    at endReadableNT (node:internal/streams/readable:1312:12)\n    at processTicksAndRejections (node:internal/process/task_queues:83:21)",
    "config": {
        "transitional": {
            "silentJSONParsing": true,
            "forcedJSONParsing": true,
            "clarifyTimeoutError": false
        },
        "transformRequest": [
            null
        ],
        "transformResponse": [
            null
        ],
        "timeout": 120000,
        "xsrfCookieName": "XSRF-TOKEN",
        "xsrfHeaderName": "X-XSRF-TOKEN",
        "maxContentLength": -1,
        "maxBodyLength": -1,
        "headers": {
            "Accept": "application/json, text/plain, */*",
            "User-Agent": "axios/0.24.0"
        },
        "method": "delete",
        "url": "http://192.168.4.47/api/playground/project",
        "params": {
            "key": "********************************",
            "project": "fake_project"
        }
    },
    "status": 400
}

Fix #605.

@BryceStevenWilley BryceStevenWilley marked this pull request as ready for review December 13, 2022 21:30
Copy link
Collaborator

@plocket plocket left a comment

Choose a reason for hiding this comment

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

Good goal! We decided what we'd like to do is properly isolate the .toJSON() calls in the docassemble_api_REST.js file. That way, everything else can just throw the error without worrying about it. Only the functions in the REST file will use .toJSON().

Copy link
Collaborator

@plocket plocket left a comment

Choose a reason for hiding this comment

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

I made some notes, but I don't think they're blockers (iirc). I think this is a great change in general.

lib/docassemble/setup.js Show resolved Hide resolved
lib/docassemble/takedown.js Outdated Show resolved Hide resolved
lib/docassemble/takedown.js Outdated Show resolved Hide resolved
lib/docassemble/setup.js Outdated Show resolved Hide resolved
@BryceStevenWilley
Copy link
Collaborator Author

Needs a CHANGELOG update 😬

plocket and others added 13 commits February 15, 2023 10:47
On an example erroring docassemble server (for example, this server returning
a [502 to a delete project](https://github.com/SuffolkLITLab/docassemble-EFSPIntegration/actions/runs/3641406524/jobs/6147303425)),
the entire axios error will print, which is that case in 1.8k lines, most of it
being garbage.

Axios has a `toJSON()` method on errors that should reduce length of the errors
being shown, as it will not print the most annoying parts of the long errors,
like the internal Axios socket objects

Still need to test, as it's difficult to get a server to 502 on
demand.

Fix #605.
Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
@BryceStevenWilley
Copy link
Collaborator Author

Did a changelog update, should be ready to go!

@BryceStevenWilley BryceStevenWilley merged commit bdbe48b into releases/v4 Mar 10, 2023
@BryceStevenWilley BryceStevenWilley deleted the shorter_axios_errors branch March 10, 2023 21:03
BryceStevenWilley added a commit that referenced this pull request Mar 10, 2023
* Use Axios's `toJSON` method to make shorter errors

On an example erroring docassemble server (for example, this server returning
a [502 to a delete project](https://github.com/SuffolkLITLab/docassemble-EFSPIntegration/actions/runs/3641406524/jobs/6147303425)),
the entire axios error will print, which is that case in 1.8k lines, most of it
being garbage.

Axios has a `toJSON()` method on errors that should reduce length of the errors
being shown, as it will not print the most annoying parts of the long errors,
like the internal Axios socket objects

Still need to test, as it's difficult to get a server to 502 on
demand.

Fix #605.

* Actually fail when we log the errors
* Move `toJSON()` calls to docassemble_api_REST
* Add better comment to `process.exitCode`

Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
BryceStevenWilley added a commit that referenced this pull request Mar 13, 2023
* Use Axios's `toJSON` method to make shorter errors

On an example erroring docassemble server (for example, this server returning
a [502 to a delete project](https://github.com/SuffolkLITLab/docassemble-EFSPIntegration/actions/runs/3641406524/jobs/6147303425)),
the entire axios error will print, which is that case in 1.8k lines, most of it
being garbage.

Axios has a `toJSON()` method on errors that should reduce length of the errors
being shown, as it will not print the most annoying parts of the long errors,
like the internal Axios socket objects

Still need to test, as it's difficult to get a server to 502 on
demand.

Fix #605.

* Actually fail when we log the errors
* Move `toJSON()` calls to docassemble_api_REST
* Add better comment to `process.exitCode`

Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
plocket added a commit that referenced this pull request Mar 13, 2023
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.

Remove axios full error log for setup (e.g. when Project is deleted or created), like with a 504
2 participants