-
Notifications
You must be signed in to change notification settings - Fork 103
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
Replacing request library for callbacks with fetch #585
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
473dc12
to
376fd87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but it looks like there are a lot of events that we are no longer supporting (not sure if they are in the new fetch library either) - eg: socket, abort, finish, etc.
From what I could tell node-fetch did not offer the same event granularity as the request library. You do not receive the same type of events and errors will be piped to the catch/reject rather than an event. |
@@ -42,6 +42,7 @@ | |||
"@types/express": "^4.17.17", | |||
"@types/jsforce": "^1.9.33", | |||
"@types/node": "^18.11.11", | |||
"@types/node-fetch": "^2.6.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR says it's replacing the request
library. Can we remove the dependency?
Adding ability to check status codes and return proper errors to Looker