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.

Sign up for GitHub

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

Jump to bottom

request closed error tags #4542

Open
joshkel opened this issue Feb 4, 2025 · 1 comment · May be fixed by #4543
Open

request closed error tags #4542

joshkel opened this issue Feb 4, 2025 · 1 comment · May be fixed by #4543
Labels
documentation Non-code related changes

Comments

@joshkel
Copy link
Contributor

joshkel commented Feb 4, 2025 •
edited
Loading

Module version

21.3.12

What documentation problem did you notice?

According to the documentation at https://hapi.dev/api/?v=21.3.12#-request-event, an internally generated error with tags request closed error indicates that "the request closed prematurely."

In my own application, I've seen tags of request error close, which apparently (also?) indicate that the request closed prematurely. I believe that this is logged from https://github.com/hapijs/hapi/blob/v21.3.12/lib/request.js#L720. This set of tags does not appear to be documented.

Therefore, I think that one of the following is true:

  1. request error close is undocumented and should be added to API.md.
  2. request closed error is a mistake, and it's actually request error close / request close error (depending on whether tag order is intended to matter).

(I think it's the second, because I don't see in the source code where it can emit a tag of closed - but I could easily be mistaken. request error close is also more consistent with the documented response error close.)
@joshkel joshkel added the documentation Non-code related changes label Feb 4, 2025
@kanongil
Copy link
Contributor

kanongil commented Feb 5, 2025

Thanks for the report. This does indeed seem to be an unintended rename.

'closed' used to be emitted instead, as can be seen from this v16 code:

hapi/lib/request.js

Lines 185 to 192 in 467e8de

this._onClose = () => {
this._log(['request', 'closed', 'error']);
this._isPayloadPending = false;
this._isBailed = true;
};
this.raw.req.once('close', this._onClose);

Given that 'close' is now used in practice, I think the best fix will be to correct the documentation.

The ordering should probably also be updated to match, though it should not matter for processing inside hapi (but might for external tools that handle a logged line). The request abort error docs should also be updated with the new ordering.

joshkel added a commit to joshkel/hapi that referenced this issue Feb 5, 2025
@joshkel
Update event tags
7065dcd 
Fixes hapijs#4542
@joshkel joshkel linked a pull request Feb 5, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation Non-code related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update event tags in documentation joshkel/hapi
2 participants
@joshkel @kanongil