SOAP faults are no longer being passed back in the error callback/promise rejection

[wagnerch]

Based on suggestion in #1146 (comment) to fix this issue with SOAP faults.

[wagnerch]

Looks like there is more broken here from the original PR. Tests are failing with this minor change, and it appears err.response.data is not a string when requestStream is used from the http client.

[jsdevel]

@wagnerch how many tests are broken?

[wagnerch]

@jsdevel I think it's everything using requestStream, but the problem is the test head dies on the first encounter (so it never completes the tests). Oddly it is failing on a completely different test this morning, which seems unrelated because master also fails.

t$ npm run-script test

> [email protected] test /home/wagnerch/dev/node-soap
> mocha --timeout 15000 --bail --exit test/*-test.js test/security/*.js

  ✓ should allow customization of httpClient and the wsdl file download should pass through it
  ✓ should allow customization of httpClient, the wsdl file, and associated data download should pass through it
  ✓ shall generate correct header for custom defined header arguments (263ms)
  ✓ should create async client without options
  1) should add namespace to array of objects

  4 passing (15s)
  1 failing

  1) should add namespace to array of objects:
     Error: Timeout of 15000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/wagnerch/dev/node-soap/test/client-test.js)
      at listOnTimeout (internal/timers.js:557:17)
      at processTimers (internal/timers.js:500:7)

[wagnerch]

@jsdevel Apparently I had to bump the timeout to 30000 for that test, not sure what's going on there. This is what I got up to when it fails:

  SOAP Client (with streaming)
    ✓ should error on invalid host
    ✓ should detect uppercase schemas as urls
    ✓ should add and clear soap headers
    ✓ should issue async callback for cached wsdl
    ✓ should allow customization of httpClient
    ✓ should allow customization of request for http client
    ✓ should allow customization of envelope (105ms)
(node:3482) UnhandledPromiseRejectionWarning: TypeError: err.response.data.indexOf is not a function
    at /home/wagnerch/dev/node-soap/test/client-test.js:119:45
    at /home/wagnerch/dev/node-soap/src/client.ts:269:9
    at /home/wagnerch/dev/node-soap/node_modules/lodash/lodash.js:10118:25
    at finish (/home/wagnerch/dev/node-soap/src/client.ts:338:16)
    at parseSync (/home/wagnerch/dev/node-soap/src/client.ts:379:14)
    at /home/wagnerch/dev/node-soap/src/client.ts:520:20
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:3482) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 15)
(node:3482) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

    1) should allow passing in XML strings


  89 passing (60s)
  1 failing

  1) SOAP Client (with streaming)
       should allow passing in XML strings:
     Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/wagnerch/dev/node-soap/test/client-test.js)
      at listOnTimeout (internal/timers.js:557:17)
      at processTimers (internal/timers.js:500:7)

[wagnerch]

I don't really quite know the best approach for solving this problem with requestStream.

But one thing worth mentioning here, which may have detected this failure earlier with that PR, is the test cases all return http status code 200 for SOAP fault testing -- not sure why? The W3C spec says they should return http status code 500, https://www.w3.org/TR/2000/NOTE-SOAP-20000508/#_Toc478383529

For example, changing status code to 500 on master:

(node:6475) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'Request failed with status code 500'
- 'Test: test error: "test detail"'
    at /home/wagnerch/dev/node-soap/test/client-test.js:1096:20
    at /home/wagnerch/dev/node-soap/src/client.ts:269:9
    at /home/wagnerch/dev/node-soap/src/client.ts:572:9
    at /home/wagnerch/dev/node-soap/src/http.ts:213:14
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

[jsdevel]

@wagnerch please update with the latest from master. i had to migrate to github actions.

[wagnerch]

Reverted some of the failing test cases to 0.39.0, some of these were changed for the axios implementation.

• With streaming, err.response.data is a ReadStream, and the stream is already consumed by error handling. The raw is still parsed in this case, so reverted the first case back to using raw.
• Reverted the second case back to using err.body, which it was using in v0.39.0, however I had to add a JSON.stringify but I suspect this may be caused by axios itself?

I can revert the SOAP fault test case change, even thought I think it is right. That other commit passes under v0.39.0, and passes with the changes prior to these changes.

[wagnerch]

Ok, figured out the issue with the 2nd client-test. Axios by default was doing JSON parsing of the responses which is apparently different behavior than requests, disabled this by overriding transformResponse in the request config.

[kasia-sztymelska]

Hi,

When updating from 0.39.0 to 0.42.0 one of my tests is failing.
Before:
t.is(err.response.statusCode, 500)
After:
t.is(err.response.statusCode, undefined)

Looks like response.statusCode is no longer set on the error. The field response.status is set instead, but still 0.42.0 is not backwards compatible with 0.39.0.

[wagnerch]

@kasia-sztymelska statusCode was changed to status, this is caused by PR #1146.

It seems there is no test coverage for statusCode. I think the only thing that will return statusCode is a WSDL parsing issue. This is basically because in 0.39 and prior request was used and statusCode came from the object response of that code, in 0.40 and later it is using axios which uses status.

Edit: Actually, there was test coverage for statusCode, but it was changed. You will see that by reviewing the PR changes.