Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: fire an error event on middleware failure for non-root namespace #1202

Merged
merged 1 commit into from
May 17, 2018

Conversation

darrachequesne
Copy link
Member

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

In the following example:

io.use((socket, next) => {
  next(new Error('Auth failed'));
});

// client-side
const socket = io('https://url/custom-namespace');

socket.on('error', (err) => {
  // event was not fired
});

The 'error' event wasn't fired on the custom namespace.

New behaviour

The event is now fired.

Other information (e.g. related issues)

Related:

In the following example:

```js
io.use((socket, next) => {
  next(new Error('Auth failed'));
});

// client-side
const socket = io('https://url/custom-namespace');

socket.on('error', (err) => {
  // ...
});
```

The 'error' event wasn't fired on the custom namespace.
@darrachequesne darrachequesne merged commit 165ce3e into socketio:master May 17, 2018
@darrachequesne darrachequesne deleted the fix/middleware-auth branch May 17, 2018 20:51
darrachequesne added a commit that referenced this pull request May 17, 2018
#1202)

In the following example:

```js
io.use((socket, next) => {
  next(new Error('Auth failed'));
});

// client-side
const socket = io('https://url/custom-namespace');

socket.on('error', (err) => {
  // ...
});
```

The 'error' event wasn't fired on the custom namespace.
@darrachequesne darrachequesne added this to the 2.1.1 milestone Nov 29, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant