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

Error catch causes error that hides the real error. #229

Closed
robblovell opened this issue Jun 22, 2021 · 2 comments · Fixed by #293 · May be fixed by #231
Closed

Error catch causes error that hides the real error. #229

robblovell opened this issue Jun 22, 2021 · 2 comments · Fixed by #293 · May be fixed by #231

Comments

@robblovell
Copy link

robblovell commented Jun 22, 2021 •
edited

Also, there is a problem in the client code:

Here:

 async request(method, path, options = {}) {
    try {
   ...
    } catch (error) {
      let clientError;
      try {
        const response = JSON.parse(error.response.body);
        clientError = new NgrokClientError(
          response.msg,
          error.response,
          response
        );
      } catch (e) {
        clientError = new NgrokClientError(
          error.response.body,
          error.response,
          error.response.body
        );
      }
      throw clientError;
    }
  }

Some errors don't have a response or a response.body and you "typeerror cannot read property 'body' of undefined", probably from the second catch... It would be better to just return the error. When doing so, you get a much more useful message that helped to debug the above missing await (ngrok is not yet ready to start tunnels):

in client.js

 async request(method, path, options = {}) {
    try {
   ...
    } catch (error) {
      let clientError;
      try {
        const response = JSON.parse(error.response);
        clientError = new NgrokClientError(
          response.msg,
          error.response,
          response
        );
      }
robblovell added a commit to robblovell/ngrok that referenced this issue Jun 22, 2021
@robblovell
fixes error: typeerror cannot read property 'body' of undefined buben…
7912a20 
…shchykov#229
@robblovell robblovell mentioned this issue Jun 22, 2021
Closed
@robblovell
Copy link
Author

Perhaps there are changes in the replies from the NGROK api that are causing problems? Not sure. But it looks like there are some gremlins in here somewhere that cause the error handling to crash and the retry logic to fail.

Periodically, ECONNREFUSED 127.0.0.1:4040 is returned.

@robblovell robblovell mentioned this issue Jun 23, 2021
Open
@sarink sarink mentioned this issue Jan 27, 2022
Closed
@gonzaloriestra
Copy link

I'm facing the same issue. I can reproduce it by updating my Ngrok token to a wrong one, and then I get the Cannot read property 'body' of undefined. But it doesn't happen every time, sometimes I get NgrokClientError: failed to start tunnel as expected.

philnash added a commit that referenced this issue Aug 16, 2022
@philnash
Throw client errors correctly.
1f0f66e 
If the HTTP client catches an error but it doesn't have a response, then it is not an HTTP error, so we'll throw the original error instead. This will surface what is causing an error that isn't because of an HTTP response in the client.

Should fix #229, or at least help diagnose it.
@philnash philnash mentioned this issue Aug 16, 2022
Merged
bubenshchykov pushed a commit that referenced this issue Mar 13, 2023
@philnash
Throw client errors correctly. (#293)
c95c089 
If the HTTP client catches an error but it doesn't have a response, then it is not an HTTP error, so we'll throw the original error instead. This will surface what is causing an error that isn't because of an HTTP response in the client.

Should fix #229, or at least help diagnose it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@robblovell @gonzaloriestra