uap-go icon indicating copy to clipboard operation
uap-go copied to clipboard

Stop using MustCompile, and instead return an error if any regex fails to compile

Open veqryn opened this issue 4 years ago • 4 comments

The use of regex.MustCompile (see: https://github.com/ua-parser/uap-go/blob/daf92ba38329329419decc8e322bcd9480964294/uaparser/parser.go#L354-L360 ) means that if any regex fails, there is a panic.

We pull down the master regex url daily, so that our regexes are always up to date, and load it right into our servers. The panic cause all our servers to crash last night, until we pinned the master regex url to a working version.

NewFromBytes already has the option to return an error, so why not return the error there? https://github.com/ua-parser/uap-go/blob/daf92ba38329329419decc8e322bcd9480964294/uaparser/parser.go#L209-L219

veqryn avatar Jan 29 '20 23:01 veqryn

I agree! @elsigh would you consider a merge request changing the panics to correct error handling (also in NewFromSaved)?

(it's a breaking change)

sylvain101010 avatar Jul 29 '20 14:07 sylvain101010

regex.MustCompile 的使用(见:

https://github.com/ua-parser/uap-go/blob/daf92ba38329329419decc8e322bcd9480964294/uaparser/parser.go#L354-L360

) 意味着如果任何正则表达式失败,就会出现恐慌。 我们每天下拉主正则表达式网址,以便我们的正则表达式始终是最新的,并将其直接加载到我们的服务器中。恐慌导致我们所有的服务器昨晚崩溃,直到我们将主正则表达式 url 固定到一个工作版本。

NewFromBytes 已经可以选择返回错误,那么为什么不在那里返回错误呢?

https://github.com/ua-parser/uap-go/blob/daf92ba38329329419decc8e322bcd9480964294/uaparser/parser.go#L209-L219

I want to ask you if this error will cause the program to hang up directly without any error log. My program also uses client: = parser.parse, and hang up directly without any error log during operation, which makes me wonder where there is a bug

yehaotong avatar Aug 24 '21 11:08 yehaotong

I'm happy to accept a breaking change, but one issue really is that I'm not using this library in prod atm so I'd rather find someone else who can manage/own such a change.. Interested @skerkour ?

elsigh avatar Aug 24 '21 13:08 elsigh

Hello, Unfortunately I absolutely don't have the bandwidth available to own the change.

Kind regards

sylvain101010 avatar Sep 14 '21 15:09 sylvain101010