BorisOsipov / wdio-reportportal-reporter

A WebdriverIO plugin. Report results to Report Portal.
MIT License
23 stars 29 forks source link

[Feature] Add `throwOnReportingError` option #212

Closed ntkabalan closed 1 year ago

ntkabalan commented 1 year ago

Proposed changes

Sometimes we see bad responses from our Report Portal instance, due to regressions, database issues, etc. In that case, we'd like to avoid failing an entire test suite because the reporter is having issues.

This PR adds a new reporter option — throwOnReportingError. When set to true, exceptions will be allowed to bubble up to the testing suite using this reporter. This is the default behavior. When set to false, exceptions will be caught, logged, and suppressed.

BorisOsipov commented 1 year ago

Hi @ntkabalan,

Thank you for submitting the pull request. Unfortunately, I am going to reject it because I believe this is not the proper place to try/catch errors in the reporter. If you do try/catch and suppress the errors in any way, you will not have a proper report. It seems strange to me to run tests but give up on the test results. It is probably better to fix your RP performance issues if you can, or get rid of RP if you can't.

Additionally, you can file an issue on the main wdio repo and request such a feature for all wdio-based reporters. There may be other opinions that differ from mine.

As a workaround, you can extend the reporter class in your repo and do whatever you want, for example:

import ReportPortalReporter from "wdio-reportportal-reporter";

class MySafeRpReporter extends ReportPortalReporter {
    onSuiteStart(suite) {
        try {
            super.onSuiteStart(suite)
        } catch (e) {
            //any logic you want.
        }
    }
}

and then in wdio config

const safereportportal = require('./MySafeRpReporter.js');

exports.config = {
  // ...
  reporters: [[safereportportal, conf]],
  // ...
};

Also, you don't need to try/catch a static method of the reporter like public static addAttribute(attribute: Attribute) because they cannot throw exceptions related to performance/network issues.

If you have any other questions, I am open to answering them.

ntkabalan commented 1 year ago

@BorisOsipov Understood. We're going to take an approach similar to your MySafeRpReporter suggestion. Thank you for the insight, and for all of your OSS work!