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

Avoid using self in service.ts - to avoid issue with through #95

Merged

Conversation

willstott101
Copy link
Contributor

@willstott101 willstott101 commented Jan 24, 2022

I've been seeing errors like this during shutdown of my integration test suite.

  ● auth › repo push auth test allowed repo, specific branches

    TypeError: _this.queue is not a function

      at Stream.call (node_modules/node-git-server/dist/service.js:115:37)
      at Stream.stream.write (node_modules/through/index.js:26:11)

by switching to using the through stream via closure seems to fix the issue. Whilst in there I went ahead and tidied up the usage of self as I believe when operating inside closures of a class method... this should always refer to the same thing. By removing the self we can avoid temptation to re-define this in any closure.

}
respStream.queue(Buffer.from('0000'));
respStream.queue(null);
}

self.emit.bind(self, 'exit');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also went ahead and fixed this no-op.

Copy link
Owner

@gabrielcsapo gabrielcsapo left a comment

Choose a reason for hiding this comment

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

This is awesome thank you!

@gabrielcsapo gabrielcsapo merged commit 73ec48e into gabrielcsapo:master Jan 25, 2022
@gabrielcsapo
Copy link
Owner

Published as 1.0.0-beta.2.

gabrielcsapo added a commit that referenced this pull request Jul 11, 2022
- Removes node support from node@<16 (@gabrielcsapo)
- Bugfix: Fix logging on response streams. (#96) (@willstott101)
- Avoid using self in service.ts - to avoid issue with through (#95) (@willstott101)
- Migrates to typescript (@5GameMaker @gabrielcsapo)
- Removes node support from node@<14
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.

2 participants