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

[Client JS] traceSDKInternal may not working、fetch.ts should handle exception、exitspan's url tag may be an empty stirng #7335

Closed
2 of 4 tasks
Lighfer opened this issue Jul 19, 2021 · 1 comment · Fixed by apache/skywalking-client-js#68
Assignees
Labels
bug Something isn't working and you are sure it's a bug! Client JS Browser agent

Comments

@Lighfer
Copy link
Contributor

Lighfer commented Jul 19, 2021

Please answer these questions before submitting your issue.

  • Why do you submit this issue?
  • Question or discussion
  • Bug
  • Requirement
  • Feature or performance improvement

Bug

traceSDKInternal may not working

The hasTrace variable in the fetch.ts and xhr.ts may have wrong value, for example:

ClientMonitor.register({
  collector: 'http://IP/skywalking',
  traceSDKInternal: false
  ...
})

the collector's pathname is /skywalking, so the segments report request will send to http://IP/skywalking/v3/segments, and it's pathname is /skywalking/v3/segments, so the result of following code will be false:

([ReportTypes.ERROR, ReportTypes.ERRORS, ReportTypes.PERF, ReportTypes.SEGMENTS] as string[]).includes(
  url.pathname,
)

fetch.ts should handle exception

// fetch.ts
const response = await origFetch(...args); // should wrap with try catch to ensure that the error log and segment  can be collected

Requirement or improvement

When the request send failed, The exitSpan's url tag in the fetch.ts and xhr.ts may be an empty string, such as the cors error. So i think it can be replaced with the request's url when the responseURL is empty.

// xhr.ts or fetch.ts
const exitSpan: SpanFields = {
  ...
  tags: options.detailMode
    ? [
        ...
        {
          key: 'url',
          value: segCollector[i].event.responseURL // may be an empty string
          // maybe replae wih: segCollector[i].event.responseURL || `${url.protocol}//${url.host}${url.pathname}`
        },
      ]
};
@wu-sheng
Copy link
Member

Would you send a pull request to fix?

@wu-sheng wu-sheng added Client JS Browser agent bug Something isn't working and you are sure it's a bug! labels Jul 19, 2021
@wu-sheng wu-sheng added this to the ClientJS - 0.7.0 milestone Jul 19, 2021
Lighfer pushed a commit to Lighfer/skywalking-client-js that referenced this issue Jul 19, 2021
Fine0830 pushed a commit to apache/skywalking-client-js that referenced this issue Jul 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working and you are sure it's a bug! Client JS Browser agent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants