customInternalAttach -> authorize -> errCallback undefined?

[ryzr]

Echo Version

1.0.3

Laravel Version

10.38.1

PHP Version

8.2

NPM Version

10.1

Database Driver & Version

MySQL 8.0.35 on AWS RDS

Description

Just a minor one, but gets a bit spammy in Sentry. There appears to be cases where the overriden channel attach method is called with an undefined errCallback. It doesn't appear to have any impact on the end user, but it does get reported in Sentry at least a few times a day

https://github.com/ably-forks/laravel-echo/blob/master/src/channel/ably/attach.ts#L28

Wasn't sure if it would be fine to check if errCallback is defined before calling, or something more elaborate like this would be necessary:

https://github.com/ably/ably-js/blob/fdd5a566eafac0a7fae9592f890361a608794487/src/common/lib/client/realtimechannel.ts#L335-L339

Merry Christmas! 🎄

Steps To Reproduce

I haven't been able to reproduce this myself, but looking at the Sentry replays, it seems that users are idling, a request is made which 403s, and then the error above occurs.

[sync-by-unito]

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-4008

[sacOO7]

@ryzr Thanks for reporting, seems we can modify check as

if (error && errCallback)

[sacOO7]

Since this is a small change, you might like to create a PR for the same 👍

[sacOO7]

@ryzr I have merged the PR. We will see if we can do the release in the near future. If you want to use the latest code on the master branch, you can include github url for the same in package.json dependency.
https://dev.to/michalbryxi/github-urls-in-package-json-5412

[sacOO7]

Seems we need to fix this properly, so I opened #31

[sacOO7]

@ryzr please do add comments there if you find anything missing

[ryzr]

Thanks @sacOO7 😄

[sacOO7]

@ryzr I have merged the PR. We will see if we can do the release in the near future. If you want to use the latest code on the master branch, you can include github url for the same in package.json dependency. https://dev.to/michalbryxi/github-urls-in-package-json-5412

@ryzr do update the code as per comment and let us know if you still facing issues with sentry.

[sacOO7]

@ryzr please do star https://github.com/ably/laravel-broadcaster and this repo if your issue is addressed 👍

[ryzr]

Have done - thanks again for all your help, I appreciate it! :-)