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

Enforcement: stop importing events system in bidders #11071

Closed
patmmccann opened this issue Feb 7, 2024 · 5 comments
Closed

Enforcement: stop importing events system in bidders #11071

patmmccann opened this issue Feb 7, 2024 · 5 comments
Assignees

Comments

@patmmccann
Copy link
Collaborator

patmmccann commented Feb 7, 2024

Type of issue

feature request

Description

to prevent the need for any adaptor to import the events system, we should expand the scope of onBidderError to more than just network errors, potentially to support use cases such as failing to understand the response or something of this nature.

The following bidders currently import events

import * as events from '../src/events.js';
fyi @kdesput please convert to use onBidWon

We should add the linter rule

@dgirardi
Copy link
Collaborator

dgirardi commented Feb 7, 2024

Need to define what the extended scope would be. Bidders have control over their code and can already do what they wish with errors they generate. Looking at those adapters:

  • nextMillennium: their use of events is not related to errors - it looks like plain analytics
  • holid: not related to errors, it should be using onBidWon

This leaves ix. they intend to capture error logs and rendering failures related to their adapter. I don't think their approach works, but besides that:

  • an error message should always come together with an error. It makes more sense to look at the error - we could provide them an onException handler, but they could also just wrap their logic a try/catch.
  • rendering failures are, to the best of my knowledge, not bidder failures. I don't think a bidder can cause them.

@patmmccann patmmccann changed the title Feature: enhanced onBidderError Enforcement: stop using events Feb 12, 2024
@patmmccann patmmccann changed the title Enforcement: stop using events Enforcement: stop importing events system in bidders Feb 12, 2024
@patmmccann patmmccann moved this from Triage to Blocked in Prebid.js Tactical Issues table Feb 12, 2024
@patmmccann patmmccann moved this from Blocked to In progress in Prebid.js Tactical Issues table Feb 12, 2024
@patmmccann patmmccann moved this from In progress to Ready for Dev in Prebid.js Tactical Issues table Feb 12, 2024
@patmmccann
Copy link
Collaborator Author

#11141 partially fixes

@patmmccann
Copy link
Collaborator Author

docs: prebid/prebid.github.io#5189

@patmmccann patmmccann assigned mkomorski and unassigned lksharma May 9, 2024
@patmmccann
Copy link
Collaborator Author

patmmccann commented May 9, 2024

Summary of to do: fix holid adapter, add a linting rule for bidders not to import events using a no-restricted-imports rule on es lint

@dgirardi
Copy link
Collaborator

Also add linting rule to exclude bidders from importing adLoader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants