Skip to content

fix(queue): always close eclient conn if it exists#178

Merged
skeggse merged 1 commit intobee-queue:masterfrom
ethanresnick:close-fix
Jan 27, 2020
Merged

fix(queue): always close eclient conn if it exists#178
skeggse merged 1 commit intobee-queue:masterfrom
ethanresnick:close-fix

Conversation

@ethanresnick
Copy link
Copy Markdown
Contributor

Before, the eclient was created if this was true (see line 77):

this.settings.getEvents || this.settings.activateDelayedJobs

But then it was closed iff this.settings.getEvents was true, which
meant that close() would fail to terminate all connections if the
settings were { getEvents: false, activateDelayedJobs: true }.

Before, the eclient was created if this was true (see line 77):

```
this.settings.getEvents || this.settings.activateDelayedJobs
```

But then it was closed iff `this.settings.getEvents` was true, which
meant that `close()` would fail to terminate all connections if the
settings were `{ getEvents: false, activateDelayedJobs: true }`.
@ethanresnick ethanresnick requested a review from skeggse January 27, 2020 11:00
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling ffdf2af on ethanresnick:close-fix into c92207e on bee-queue:master.

@ethanresnick
Copy link
Copy Markdown
Contributor Author

@skeggse FYI, the travis failure seems unrelated to this change; looks like eslint ships some modern JS code that's not compatible with older versions of Node

@skeggse
Copy link
Copy Markdown
Member

skeggse commented Jan 27, 2020

@ethanresnick good catch, thanks!

@skeggse skeggse merged commit 66d6dc6 into bee-queue:master Jan 27, 2020
@ethanresnick
Copy link
Copy Markdown
Contributor Author

Sure thing. Thanks for merging so quickly :)

Any chance a new version can be published to npm with this fix? I need it for my current project, and would rather not have to create a distinct npm package to depend on

@ethanresnick ethanresnick deleted the close-fix branch January 27, 2020 20:04
@skeggse
Copy link
Copy Markdown
Member

skeggse commented Jan 28, 2020

Released in v1.2.3.

@ethanresnick
Copy link
Copy Markdown
Contributor Author

Thanks!

@skeggse skeggse added this to the 1.2.4 milestone Apr 16, 2020
@skeggse skeggse added the bug label Apr 16, 2020
@skeggse skeggse self-assigned this Apr 16, 2020
@skeggse skeggse modified the milestones: 1.2.4, 1.2.3 Apr 16, 2020
@skeggse skeggse linked an issue Apr 16, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workers with activateDelayedJobs without getEvents

3 participants