-
Notifications
You must be signed in to change notification settings - Fork 522
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
Introduce ConnectionErrorType enum for modeling connection errors to hosts #1581
Conversation
Updated per Ruchika's request. Need to fix the tests. |
To hopefully fix "missing" export of base class ErrorWithProps
@@ -643,13 +643,14 @@ export class DeltaManager extends EventEmitter implements IDeltaManager<ISequenc | |||
replayFrom: from, | |||
to, | |||
}); | |||
const closeError: IConnectionError = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vladsud - This is interesting - the fact that someone can just instantiate an object adhering to the interface means they miss out on extending ErrorWithProps
and Error
. Not extending Error
is probably ok since it's not thrown anyway, but it won't be logged properly without extending ErrorWithProps
. Not sure how to enforce this, something to discuss... Worthy of an issue?
undefined /*statusCode*/, | ||
undefined /*retryAfterSeconds*/, | ||
"Online", | ||
) as IGenericNetworkError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: This is sloppy, I'd like to revisit this in a later change.
Minimal fix for #1354. It's a non-breaking change so they can switch over from raw
statusCode
to the more semanticconnectionError
at their leisure.More thought could probably be put into the model here, probably in a later change:
connectionError
inNetwork.ts
?IConnectionError
up into other interfaces similarly to howIError
is defined, and replace the classNetworkError
with different specific implementations, maybe with a little factory method to abstract the differences from callers.Also, I might propose some refactoring of
OdspNetworkError
in another change, it's also modeled a little strangely (e.g. passing in fake400
status codes).