fix buffer concatenation & add support for some more API endpoints#6
Open
ceejbot wants to merge 12 commits intodyninc:masterfrom
Open
fix buffer concatenation & add support for some more API endpoints#6ceejbot wants to merge 12 commits intodyninc:masterfrom
ceejbot wants to merge 12 commits intodyninc:masterfrom
Conversation
there is a problem when the call to obtain the status of a sender return false, the callback for the promise is never reached because the errorCase function is throwing an exception that never is catched
with my changes now no matter the result of the status, the respond is always reached
function to validate the new changes propossed
++++++++++++++++
function isAvalidSender(email) {
var result = dyn.senders.status(email).then(function(x) {
console.log('CALLBACK!!!');
console.log('response %s', JSON.stringify(x));
if (x) {
return true;
}
return false;
});
}
I forgot to remove try catch
in order to no affect other operations, I just added throwResponse definition and changed the call for messaging.senders.status
messaging.senders.status = (email) -> callWithError messaging.senders._status._call(messaging, {}, _.defaults({emailaddress:email}, {apikey:messaging.defaults.apikey})), "senders.status", msgIsOk, extractMsgData, throwResponse
the level of impact is minor and the response is reached by the client whom call the action
response {"status":404,"message":"Not Found","data":{"emailaddress":"whatever@veo.tv"}}
response {"ready":1}
No more uncaught exceptions on JSON parse
merge in elopezanaya's fork
Added coffee-script to the dev dependencies. Added an npm script target to run it, in lieu of the ruby one-liner. Ran it on the coffeescript source with pull requests merged.
Now we concatenate buffers from readable streams properly, and we handle the errors they might emit, who knows. JSON.parse() can throw exceptions. Catch them.
The npm `compile` script target does all that's needed here.
If x.data is an array, we want to parse it as a list of strings. If x.data is already an object, we want to return it. This makes API calls like `dyn.record._CNAME.get()` actually work. While I was there, bumped deps.
JakeChampion
approved these changes
Jun 20, 2017
JakeChampion
approved these changes
Jun 20, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npm, Inc uses Dyn as its DNS provider, and I recently wrote a command-line tool to help us automate some operational tasks that require DNS changes. I made the following modifications to our fork of the module to make this tool function.
concat-streamto properly concatenate buffers to build a request resultGET /REST/Job/<id>API endpointDELETE /REST/Node/<zone>/<fqdn>/API endpointnpm run compileto compile the single source file