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

fix: PFG-3233. Increasing minimum timeout to be not less than 1500 ms. #192

Conversation

AndriiAndrushkoFS
Copy link

@AndriiAndrushkoFS AndriiAndrushkoFS commented Jul 29, 2024

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

Other information

src/ajax.js Outdated
@@ -67,7 +67,9 @@ export function fetcherFactory(timeout = 3000, {request, done} = {}) {
let fetcher = (resource, options) => {
let to;
if (timeout != null && options?.signal == null && !config.getConfig('disableAjaxTimeout')) {
to = dep.timeout(timeout, resource);
const minTimout = 1500;
const normalizedTimeout = timeout > minTimout ? timeout : minTimout;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we can simplify that:

Suggested change
const normalizedTimeout = timeout > minTimout ? timeout : minTimout;
const normalizedTimeout = Math.max(timeout, minTimout);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanx, that's a good option.
For me looks if I simplify it this way, then it would add the need to leave a comment to explain why we using Math.max to filter out too little timeout.
I would say original line looks more self explanatory

@aecook aecook self-requested a review July 31, 2024 18:12
Copy link

@aecook aecook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately this shouldn't be be in Prebid... its fine for testing purposes but this would need to be moved to pubfig if it has some results...

@AndriiAndrushkoFS AndriiAndrushkoFS deleted the PFG-3233-requests-to-s2s-endpoint-getting-canceled-fix-V1 branch August 2, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants