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

Fixing issue #613: Add capability to throw Fault from async function #614

Closed
wants to merge 5 commits into from

Conversation

ghughal
Copy link

@ghughal ghughal commented Mar 27, 2015

No description provided.

@brodin
Copy link
Contributor

brodin commented Apr 28, 2015

What is the status of this? Seams like @ghughal have made a great effort to make these server handlers useful. Async errors are very common in node.js and needs to be supported. Looked over the commits and it seams like a no-brainer - even thou the callback convention is broken by having cb(data, err) rather than cb(err, data). Solveing https://github.com/vpulim/node-soap/issues/613

@brodin
Copy link
Contributor

brodin commented Apr 29, 2015

I did my own take on this and found out why the tests failed #641

@ghughal
Copy link
Author

ghughal commented Apr 29, 2015

@brodin - I too am waiting for someone to merge it. It would be great if someone can look at it and merge it or provide feedback.

@simonjosefsson
Copy link

I'm not a maintainer but you need to do the following first

  • Read and follow CONTRIBUTING (squashing the commits is a must)
  • Have the tests pass (run npm test locally and see the logs on Travis)

@herom
Copy link
Contributor

herom commented May 19, 2015

Hello @ghughal - thanks a lot for you contribution and sorry for the extremely long delay 😞

As @simonjosefsson said, please take a look at our Guideline on Submitting a Pull Request as there are some points which must be necessarily fulfilled in order to get your PR merged.

The main points are (as already mentioned by @simonjosefsson :

  • Rebase/Squash your commits into a single one (more information about this is available in the guideline)
  • run npm test locally and take a look at the logs at Travis CI

thanks 👍

@jsdevel
Copy link
Collaborator

jsdevel commented Sep 16, 2015

Closing for lack of activity.

@jsdevel jsdevel closed this Sep 16, 2015
@spawnrider
Copy link

Even you can't throw a Fault from an async function, you can use the callback method in order to return it like this :

    db.any("select * from users where active=$1", [true])
      .then(function (data) {
        cb({});
      })
      .catch(function (error) {
        cb({
          Fault: {
            faultcode: faultCode,
            faultstring: faultString,
            detail: {
              "ns:FaultInformation": {
                "ns:FaultCode": detailedFaultCode,
                "ns:FaultText": detailedFaultText
              }
            },
            statusCode: 500
          }
        });
      });
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants