Display error banner when Reports server returns status code ≠ 2xx

[vincent-psarga]

Other Cucumber implementations display the content send by the server upon failure. For example, with Cucumber ruby:

$ CUCUMBER_PUBLISH_TOKEN=plop bundle exec cucumber features/docs/getting_started.feature
WARNING: Nokogiri was built against LibXML version 2.9.10, but has dynamically loaded 2.9.4
Using the default profile...
........

2 scenarios (2 passed)
8 steps (8 passed)
0m0.023s
┌──────────────────────────────────────────────────────────────┐
│ Invalid CUCUMBER_PUBLISH_TOKEN: plop                         │
│ Go to https://reports.cucumber.io/profile to get your token. │
└──────────────────────────────────────────────────────────────┘
Traceback (most recent call last):
	1: from /Users/vincent.pretre/dev/open-source/cucumber-ruby-meta/cucumber-ruby/lib/cucumber/formatter/io.rb:40:in `block (3 levels) in new'
/Users/vincent.pretre/dev/open-source/cucumber-ruby-meta/cucumber-ruby/lib/cucumber/formatter/http_io.rb:79:in `close': request to https://messages.cucumber.io/api/reports failed with status 401 (StandardError)

This is an attempt to get the same behavior in cucumber-js

TODO:

• Add a example of what's excepted
• Display the server response
• Fail when the server does not return 2xx
• Solve the null line displayed above the banner

[vincent-psarga]

Seems to work fine now (except for the 'null' line that I have to fix)

CUCUMBER_PUBLISH_TOKEN=plop node ./bin/cucumber-js features/plop.feature
1 scenario (1 passed)
0 steps
0m00.016s (executing steps: 0m00.008s)
null
┌──────────────────────────────────────────────────────────────┐
│ Invalid CUCUMBER_PUBLISH_TOKEN: plop                         │
│ Go to https://reports.cucumber.io/profile to get your token. │
└──────────────────────────────────────────────────────────────┘


/Users/vincent.pretre/dev/open-source/cucumber-js/src/formatter/http_stream.ts:93
              new Error(`${method} ${url} returned status ${res.statusCode}`)
              ^
Error: GET https://messages.cucumber.io/api/reports returned status 401
    at IncomingMessage.<anonymous> (/Users/vincent.pretre/dev/open-source/cucumber-js/src/formatter/http_stream.ts:93:15)
    at IncomingMessage.emit (events.js:326:22)
    at IncomingMessage.EventEmitter.emit (domain.js:483:12)
    at endReadableNT (_stream_readable.js:1241:12)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)

[coveralls]

Coverage increased (+0.5%) to 98.575% when pulling 2e744ae on do-not-fail-when-report-server-returns-error into 4811647 on master.

[vincent-psarga]

The build is not really stable (I managed to get it green by relaunching - and after it failed again).

Apparently that comes from some tests I have added and the way the stream errors are handled. I'm going to dig a bit more into that (although the solution I have in mind is not that beautiful to be honest :D )

[aslakhellesoy]

@vincent-psarga are the non-deterministic tests new tests in this PR? I can help have a look at this.

[davidjgoss]

The flaky tests I think started from the original change where publish support was added per #1457

I tried a few things a while ago but hit a bit of a wall tbh.

[vincent-psarga]

@vincent-psarga are the non-deterministic tests new tests in this PR? I can help have a look at this.

At least from what I see, it's the test that I've introduced which is flaky. I'm looking at a way to not spawn Cucumber in order to fix this (worked fine with Ruby getting rid of spawning, so I give a try)

[aurelien-reeves]

Just tried to remove the delays in the server
The error went back

So it seems the combination of the delays, and the ports, stabilized the tests.

[aurelien-reeves]

@vincent-psarga @aslakhellesoy @vincentcapicotto Beside the race condition / inconsistent test, something still need to be done before merging that PR?

[aurelien-reeves]

@davidjgoss do you have any new feedback regarding that PR?
We would like to merge it and release it.

I plan to open a new PR dedicated to the inconsistent test once that one is merged.

[davidjgoss]

@aurelien-reeves I'll review this morning, thanks for the nudge!

[davidjgoss]

LGTM