This repository has been archived by the owner on May 22, 2020. It is now read-only.
RUN-4017/services #113
Merged
RUN-4017/services #113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rdepena
suggested changes
Apr 17, 2018
src/api/services/channel.ts
Outdated
| const mainAction = this.subscriptions.has(action) | ||
| ? this.subscriptions.get(action) | ||
| : (payload: any, id: ServiceIdentity) => this.defaultAction(action, payload, id); | ||
| const a = this.preAction ? await this.preAction(action, payload, senderIdentity) : payload; |
There was a problem hiding this comment.
lets get some meaningful variable names here.
| this.serviceMap.set(serviceIdentity.uuid, channel); | ||
| return channel; | ||
| } catch (e) { | ||
| throw new Error(e.message); |
There was a problem hiding this comment.
Two questions:
-
Why not
throw e? -
if we are not handling the error in any meaningful way, why catch at all ?
There was a problem hiding this comment.
What we are catching is a nack from the core which is coming through as a RuntimeError, when this could very well be an intentional error from the user (ie authenticated Service). I'm catching it an throwing it back as a regular error because in those cases it is not a runtime error.
src/api/services/index.ts
Outdated
| } | ||
|
|
||
| public async register(): Promise<Provider> { | ||
| try { |
There was a problem hiding this comment.
we can do without this try/catch.
src/api/services/channel.ts
Outdated
| } | ||
|
|
||
| public register(topic: string, listener: Action) { | ||
| //TODO create map of subscriptions |
rdepena
added a commit
that referenced
this pull request
May 16, 2018
* RUN-3996 fixed a multi-runtime regression (#109) * V bump.30.2 (#110) (#111) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3999 multi runtime tests config update (#112) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3999 Update multi runtime tests config * Bug/run 3965 multi runtime cache clearing issue (#114) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3965 Fix EPERM: operation not permitted * RUN-3965 Fix some cache folders not deleted issue * RUN-4017/services (#113) * passing tests * made changes * RUN-4028 - Async add listener (#115) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * some test fixes * more timeouts in tests * altered tests to check for awaiting newlistener * undo unnecessary edit * fix types of registereventlistener and unregister * hold, fixed removeAllListeners but need to test * added some tests * added testing * cleanup * fix prependonce * bit more testing * revert test changes * changed class name, some cleanup * make tests match new best practices * name of test file * tests cleanup * back to arrows for tslint * bind rawsend (#116) * RUN-4045 register window name early in creation (#117) * RUN-4033 plugin.import accepts a string (#118) * Feature/test sledghammer (#119) * sledghammer approach to fixing tests. * taking a sledghammer to our Multi runtime tests. * Moving back to es6 untill we hit a major version. * A few items from the code review. * code review items. * code review items. * V bump.30.2 (#110) (#121) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3898 - External services tests (#120) * hiold * hold * tests for external services re RUN-3898 * clean up * Test fixed, no longer leaving around runtime (#122) * test fixed, no longer leaving around runtime * cleanup * Fixed issue with the webpack grunt task where it was not reporting the errors correctly (#123) * Fixed issue with the webpack grunt task where it was not reporting the errors correctly * Passing error codes instead of arrays. * version bump. (#124)
Merged
rdepena
added a commit
that referenced
this pull request
May 16, 2018
* RUN-3996 fixed a multi-runtime regression (#109) * V bump.30.2 (#110) (#111) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3999 multi runtime tests config update (#112) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3999 Update multi runtime tests config * Bug/run 3965 multi runtime cache clearing issue (#114) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3965 Fix EPERM: operation not permitted * RUN-3965 Fix some cache folders not deleted issue * RUN-4017/services (#113) * passing tests * made changes * RUN-4028 - Async add listener (#115) * V bump.30.2 (#110) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * some test fixes * more timeouts in tests * altered tests to check for awaiting newlistener * undo unnecessary edit * fix types of registereventlistener and unregister * hold, fixed removeAllListeners but need to test * added some tests * added testing * cleanup * fix prependonce * bit more testing * revert test changes * changed class name, some cleanup * make tests match new best practices * name of test file * tests cleanup * back to arrows for tslint * bind rawsend (#116) * RUN-4045 register window name early in creation (#117) * RUN-4033 plugin.import accepts a string (#118) * Feature/test sledghammer (#119) * sledghammer approach to fixing tests. * taking a sledghammer to our Multi runtime tests. * Moving back to es6 untill we hit a major version. * A few items from the code review. * code review items. * code review items. * V bump.30.2 (#110) (#121) * RUN-3996 fixed a multi-runtime regression (#109) * Version bump to 30.2 * RUN-3898 - External services tests (#120) * hiold * hold * tests for external services re RUN-3898 * clean up * Test fixed, no longer leaving around runtime (#122) * test fixed, no longer leaving around runtime * cleanup * Fixed issue with the webpack grunt task where it was not reporting the errors correctly (#123) * Fixed issue with the webpack grunt task where it was not reporting the errors correctly * Passing error codes instead of arrays. * version bump. (#124)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Passed tests on local machine