🐛 Handle non-object response and error

[nulrich]

Motivation

We have a lot of telemetry errors concerning Fetch error that are not an Error object but a string or undefined.

Changes

• Handling the non-object Fetch Error and Response
• Supporting abort status from AbortController

Testing

• Local
• Staging
• Unit
• End to end

[cit-pr-commenter]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	161.66 KiB	161.73 KiB	71 B	+0.04%	✅
Logs	57.97 KiB	58.04 KiB	71 B	+0.12%	✅
Rum Slim	110.18 KiB	110.25 KiB	71 B	+0.06%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
addglobalcontext	0.002	0.003	0.001
addaction	0.041	0.070	0.029
adderror	0.039	0.075	0.036
addtiming	0.001	0.002	0.001
startview	1.074	4.312	3.238
startstopsessionreplayrecording	0.911	4.478	3.567
logmessage	0.020	0.092	0.071

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	20.17 KiB	20.86 KiB	706 B
addaction	72.30 KiB	74.60 KiB	2.31 KiB
adderror	86.98 KiB	87.64 KiB	677 B
addtiming	18.06 KiB	19.48 KiB	1.43 KiB
startview	348.96 KiB	355.19 KiB	6.23 KiB
startstopsessionreplayrecording	16.98 KiB	13.80 KiB	-3255 B
logmessage	69.84 KiB	70.96 KiB	1.12 KiB

🔗 RealWorld

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.65%. Comparing base (4b853b4) to head (bdc7b51).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2860      +/-   ##
==========================================
- Coverage   93.67%   93.65%   -0.02%     
==========================================
  Files         266      266              
  Lines        7586     7585       -1     
  Branches     1688     1688              
==========================================
- Hits         7106     7104       -2     
- Misses        480      481       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BenoitZugmeyer]

💭 thought: ‏Thank you for doing this! I don't think there is a good reason to have all those checks. I think we could remove them, this would allow a simpler approach:

Suggested change

	responsePromise.then(
	monitor((response) => reportFetch(response)),
	monitor((error) => reportFetch(error, true))
	)
	function reportFetch(partialContext: Partial<FetchResolveContext>) {
	const context = startContext as unknown as FetchResolveContext
	context.state = 'resolve'
	assign(context, partial)
	observable.notify(context)
	}
	responsePromise.then(
	monitor((response) => {
	reportFetch({
	response,
	responsType: response.type,
	status: response.status,
	isAborted: false
	})
	}),
	monitor((error: unknown) => {
	reportFetch({
	status: 0,
	isAborted: response instanceof DOMException && response.code === DOMException.ABORT_ERR,
	error
	})
	})
	)

[nulrich]

Done, and added support for AbortController.

[amortemousque]

🥜 nitpick: ‏You could inline it

    reportFetch({
        status: 0,
        isAborted:
          context.init?.signal?.aborted || (error instanceof DOMException && error.code === DOMException.ABORT_ERR),
        error,
      })

[nulrich]

Done

[amortemousque]

It could be nice to have a test with AbortSignal :)

[nulrich]

Done

[amortemousque]

👏 praise: ‏Very nice simplification!!

[amortemousque]

❓ question: ‏Just out of curiosity, do you know how this is transpiled in ES5? :)

[BenoitZugmeyer]

https://www.typescriptlang.org/play/?target=0#code/O4SwdgJg9sB0BmBTALgYwBYAIC8mAUesRA2gDSbgjIC6AlDgHyYDeAvrUA

Though, this syntax is a bit surprising :D

Suggestion:

window.fetch = (_input, init) => {
  ..
}

[thomas-lebeau]

It makes sense when you think about it, but I prefer benoit's suggestion (so I don't have to think about it 😄)

Or event (input, init) => { with args-after-used

[BenoitZugmeyer]

https://www.typescriptlang.org/play/?target=0#code/O4SwdgJg9sB0BmBTALgYwBYAIC8mAUesRA2gDSbgjIC6AlDgHyYDeAvrUA

Though, this syntax is a bit surprising :D

Suggestion:

window.fetch = (_input, init) => {
  ..
}