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

doc: add error documentation #789

Closed
wants to merge 5 commits into from
Closed

doc: add error documentation #789

wants to merge 5 commits into from

Conversation

chrisdickinson
Copy link
Contributor

This adds error documentation, covering "JS errors" and "system errors." It is roughly a continuation of this PR, with some remaining comments cleaned up.

R=@trevnorris

opportunity to **intercept** this error based on how the API **propagates** it.

The style of API called determines how generated errors are handed back, or
**propagated**, to client code, which in turn informs how client may **intercept**
Copy link
Contributor

Choose a reason for hiding this comment

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

"how clients" or "how the client"

@trevnorris
Copy link
Contributor

@chrisdickinson amazing amount of information here. Thanks for the docs. I would like another set of eyes on it, but overall LGTM.


<!--type=misc-->

Errors generated by Node.js fall into two categories: JavaScript errors and system
Copy link
Member

Choose a reason for hiding this comment

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

Node -> io.js ?
other api docs were updated in #750

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed most of these references. Retained "node style callback," since it seems disingenuous for io.js to claim credit for that pattern :)


Instantiates a new Error object and sets its `.message` property to the provided
message. Its `.stack` will represent the point in the program at which `new Error`
was called. Stack traces are subject to [V8's stack trace API](https://code.google.com/p/v8/wiki/JavaScriptStackTraceApi).
Copy link
Member

Choose a reason for hiding this comment

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

If I follow this link, it says that the page has moved to https://code.google.com/p/v8-wiki/wiki/JavaScriptStackTraceApi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

@chrisdickinson
Copy link
Contributor Author

Thanks @trevnorris.

@geek care to take a look?


#### ETIMEDOUT: Operation timed out

An connect or send request failed because the connected party did not properly
Copy link
Member

Choose a reason for hiding this comment

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

"A connect"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@Fishrock123
Copy link
Contributor

LGTM, this is super useful!

The style of API called determines how generated errors are handed back, or
**propagated**, to client code, which in turn informs how client may **intercept**
the error. Exceptions can be intercepted using the `try / catch` construct;
other propagation strategies are covered [below](#errors_error_propagation_and_interception).
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is linking properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears to be working locally for me.

@chrisdickinson
Copy link
Contributor Author

cc @rvagg – want to take a look and see if this is good to go for v1.2.0?

@rvagg
Copy link
Member

rvagg commented Feb 11, 2015

@chrisdickinson only a quick scan but this looks good to me, we can iterate post-merge to improve if needed IMO. Get it in quick cause I've got the release commit on my computer ready to push.

@chrisdickinson
Copy link
Contributor Author

Cool. Merging now.

chrisdickinson added a commit that referenced this pull request Feb 11, 2015
PR-URL: #789
Reviewed-by: Rod Vagg <rod@vagg.org>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@chrisdickinson
Copy link
Contributor Author

Merged in 7e2235a

restrictions, a **system error** is generated. Client code is then given the
opportunity to **intercept** this error based on how the API **propagates** it.

The style of API called determines how generated errors are handed back, or
Copy link
Member

Choose a reason for hiding this comment

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

This first line should be obvious to readers, I don't think it needs to be said.

@sam-github
Copy link
Contributor

@chrisdickinson Do I misread, or was this 5 hours from PR to merge? That's too short to get feedback, I think.

@geek
Copy link
Member

geek commented Feb 11, 2015

@chrisdickinson I added a couple of points of feedback. Amazing job, this should appeal to all developer levels.

@Fishrock123
Copy link
Contributor

@sam-github I think it was worth it to get it out and live for 1.2.0 (Also, it's docs. easy to change.)

@Fishrock123
Copy link
Contributor

@geek Mind PR-ing those?

@chrisdickinson
Copy link
Contributor Author

@Fishrock123 @sam-github It already went through a round of review here. So, five hours plus five months :) I did want to get this out with 1.2.0, hence the accelerated review + merge in the io.js case.

@geek thanks for the review!

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.

7 participants