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(AsyncQueue): call the callback only if it's a function (#2370) #2405

Merged

Conversation

athouary
Copy link
Contributor

@athouary athouary commented Dec 5, 2023

Following my PR for #2370, I ran into TypeErrors in my console where the task callback was called without being defined. The error is caught and logged so it's not critical, but it pollutes the console.

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@saghul
Copy link
Member

saghul commented Dec 5, 2023

Was the callback undefined?

@athouary
Copy link
Contributor Author

athouary commented Dec 6, 2023

Yes it was. In several places some tasks are pushed into the queue with no callback.

@saghul
Copy link
Member

saghul commented Dec 6, 2023

Then I think using optional chaining would look cleaner: func?.(arg);

@athouary
Copy link
Contributor Author

athouary commented Dec 6, 2023

I wanted to cover as many cases as possible but I guess it's overkilling it. I'm changing for optional chaining

@athouary athouary force-pushed the async-queue-task-callback-may-be-undefined branch from d9c708d to 56452eb Compare December 6, 2023 10:24
@athouary athouary force-pushed the async-queue-task-callback-may-be-undefined branch from 56452eb to 638e2c0 Compare December 6, 2023 10:24
@saghul
Copy link
Member

saghul commented Dec 6, 2023

Jenkins please test this please.

@saghul saghul merged commit f2e576a into jitsi:master Dec 14, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants